Designtime and runtime variables types...

Discussion around programming R'n'D, its source code and its tools.

Moderators: Flumminator, Zomis

Post Reply
Zomis
Posts: 1502
Joined: Mon Jun 21, 2004 1:27 pm
Location: Sweden
Contact:

Designtime and runtime variables types...

Post by Zomis »

I wonder if there's a way to make a distinction between designtime RND-variables (those used when creating levels) and runtime variables (those use for playing levels, playing around in the main menu, etc...)

There's tons of types defined in main.h, and when creating tools for RND - you don't need all of them, maybe you only need 20 percent (the designtime part) of them.
So is there a way to know which ones of the variables and types are designtime?

A script which finds all designtime variables and outputs them to a Delphi converted unit would be great ;) (Ok, just kidding - even though it would be great...)
Tomi
Posts: 339
Joined: Wed Aug 03, 2005 3:37 pm
Location: Slovakia

Post by Tomi »

A question for Holger: can I remove all the attributes marked as "internal values used at runtime when playing" in my editor? (Including, particularly, pre_change_function?)

And while I'm at asking questions, I also don't understand the comment "important: only change event bit if corresponding event flag is set" in files.c.
Tomi
Posts: 339
Joined: Wed Aug 03, 2005 3:37 pm
Location: Slovakia

Post by Tomi »

And can xx_num_contents ever get a different value than 1?
Zomis
Posts: 1502
Joined: Mon Jun 21, 2004 1:27 pm
Location: Sweden
Contact:

Post by Zomis »

"internal values used at runtime when playing" can be safely removed, yes. If you remove too much, hopefully the compiler will complain :)

xx_num_contents can get another value than one, yes. Since it's refered to as a pointer in the chunk_config_CUSX_base array.

And those bits, I'm also very confused about those...
Tomi
Posts: 339
Joined: Wed Aug 03, 2005 3:37 pm
Location: Slovakia

Post by Tomi »

Ruby is an interpreted language, so there isn't any compiler that could complain.

Here are my reasons why I believe that reading xx_num_contents always returns 1. I can't find a flaw in them, so if you can, please tell me. I'd like it confirmed by another person, because if I can assume that xx_num_contents is always 1 (when it's read), I'll be able to remove a whole bunch of code that handles it, and it will make the program much simpler, but it's a "big" decision, and if I decide to remove it but it has to be there anyway, it'll become problematic.

First, note* that xx_num_contents itself is never read. The identifier 'xx_num_contents' itself is used on these lines (of files.c):
128: it's declared, and initialized to value 0.
942: &xx_num_contents is set to num_entities in chunk_config_CUSX_base. Note that data_type = TYPE_CONTENT_LIST, default_num_values = 1 and max_num_values = 1.
1091: &xx_num_contents is set to num_entities in chunk_config_CUSX_change. Note that data_type = TYPE_CONTENT_LIST, default_num_values = 1 and max_num_values = 1.
As &xx_num_contents isn't used anywhere else in chunk_config_* variables, I think we can safely assume that when num_entities is &xx_num_contents, data_type must be TYPE_CONTENT_LIST, and both default_num_values and max_num_values must be 1.
1449: xx_num_contents is set to 1.
5124: xx_num_contents is set to 1.
So, as you can see, the program never directly reads xx_num_contents. BUT on lines 5012 and 5037 (and nowhere else, grep it yourself) it reads *num_entities, and if num_entities is &xx_num_contents, the program actually reads xx_num_contents. Line 5012 doesn't concern us, because it's inside "if (data_type == TYPE_ELEMENT_LIST)", and when num_entities is &xx_num_contents, data_type can't be TYPE_ELEMENT_LIST. So the only situation where the program can read xx_num_contents is inside SaveLevel_MicroChunk, on line 5037, and only if 'entry' is from chunk_config_CUSX_something.
Now, when does the program call SaveLevel_MicroChunk in such way? Not on line 5075, because chunk_config_INFO is used there. On line 5088 it's chunk_config_ELEM. 5105 chunk_config_NOTE. The only function where SaveLevel_MicroChunk is called with a microchunk from chunk_config_CUSX_base or chunk_config_CUSX_change is "SaveLevel_CUSX". Now, look at line 5124 in the function. It says "xx_num_contents = 1;", directly before calling SaveLevel_MicroChunk, the only function where xx_num_content is read.
So I think that logically means that always when the program reads xx_num_contents, its value is 1.

* - I'm not sure if "note" can be used like this in English, maybe "notice" would be more fitting.
Zomis
Posts: 1502
Joined: Mon Jun 21, 2004 1:27 pm
Location: Sweden
Contact:

Post by Zomis »

Indeed you have a point here..
But then the question is, why does Holger have all this code if xx_num_contents is always 1? Maybe it will be able to have more values later.

And I don't understand the comment on line 5123 (files.c)

Code: Select all

/* set (fixed) number of content areas (may have been overwritten earlier) */
So I guess that if it saves you a lot of problem to skip xx_num_contents, I guess you can skip it.
Tomi
Posts: 339
Joined: Wed Aug 03, 2005 3:37 pm
Location: Slovakia

Post by Tomi »

> But then the question is, why does Holger have all this code if xx_num_contents is always 1?
The answer is - he doesn't. He has to use some variable (because if he used NULL in the LevelFileConfigInfo, it would produce errors when the program tried to write to num_entities), so lines 128, 942 and 1091 are needed. You're partially right because line 1449 doesn't do anything (value of num_entities isn't read in SetChunkToDefaults, or what was its name). Line 5124 may be removable too, but if I'd be him, I'd probably leave it there - just to be safe. (Maybe, just maybe, it could be 0 at the moment, and then the program wouldn't write anything.)

> Maybe it will be able to have more values later.
That wouldn't make any sense. Currently he uses xx_num_contents (and not some attribute in xx_el or xx_change) because its value is always constant. If it could have different values (e.g. if a changepage could have multiple extended change targets???), he'd use a real attribute in the element change info, something like &xx_change.num_target_contents.
User avatar
Holger
Site Admin
Posts: 4073
Joined: Fri Jun 18, 2004 4:13 pm
Location: Germany
Contact:

Post by Holger »

> I wonder if there's a way to make a distinction between designtime
> RND-variables (those used when creating levels) and runtime variables
> (those use for playing levels, playing around in the main menu, etc...)

Oh well, currently there's a lot in src/main.h, so it's sometimes hard to tell (unless there are comments which directly tell what's good for what, and where). I already tried putting some things to src/game.h etc. ...

> So is there a way to know which ones of the variables and types are
> designtime?

For now, you have to "grep" for the types/values in question... A good rule of thumb is: If you find it in src/files.c, it's most probably a "design time value" (like level data), while stuff found in src/game.c is most probably runtime only.

> A question for Holger: can I remove all the attributes marked as "internal
> values used at runtime when playing" in my editor? (Including,
> particularly, pre_change_function?)

Yes, you can safely ignore these parts for editor-only code.

> And while I'm at asking questions, I also don't understand the comment
> "important: only change event bit if corresponding event flag is set" in
> files.c.

Very good point -- you're perfectly right to question this comment (found in src/files.c, function setEventBitsFromEventFlags())! Contrary to the complementary function above (setEventFlagsFromEventBits()), this comment is indeed not really true. I checked this and fixed the comments, adding some explanations. Looks like this now:

Code: Select all

static void setEventFlagsFromEventBits(struct ElementChangeInfo *change)
{
  int i;

  /* important: only change event flag if corresponding event bit is set
     (this is because all xx_event_bits[] values are loaded separately,
     and all xx_event_bits[] values are set back to zero before loading
     another value xx_event_bits[x] (each value representing 32 flags)) */

  for (i = 0; i < NUM_CHANGE_EVENTS; i++)
    if (xx_event_bits[CH_EVENT_BITFIELD_NR(i)] & CH_EVENT_BIT(i))
      change->has_event[i] = TRUE;
}

static void setEventBitsFromEventFlags(struct ElementChangeInfo *change)
{
  int i;

  /* in contrast to the above function setEventFlagsFromEventBits(), it
     would also be possible to set all bits in xx_event_bits[] to 0 or 1
     depending on the corresponding change->has_event[i] values here, as
     all xx_event_bits[] values are reset in resetEventBits() before */

  for (i = 0; i < NUM_CHANGE_EVENTS; i++)
    if (change->has_event[i])
      xx_event_bits[CH_EVENT_BITFIELD_NR(i)] |= CH_EVENT_BIT(i);
}
> And can xx_num_contents ever get a different value than 1?

Again, you've picked up a good point where my code lacks a certain degree of quality!

I will quote the whole last post again here, because it's a perfect analysis of what the code does and why (and I must say that I'm really impressed about it):
> But then the question is, why does Holger have all this code if xx_num_contents is always 1?
The answer is - he doesn't. He has to use some variable (because if he used NULL in the LevelFileConfigInfo, it would produce errors when the program tried to write to num_entities), so lines 128, 942 and 1091 are needed. You're partially right because line 1449 doesn't do anything (value of num_entities isn't read in SetChunkToDefaults, or what was its name). Line 5124 may be removable too, but if I'd be him, I'd probably leave it there - just to be safe. (Maybe, just maybe, it could be 0 at the moment, and then the program wouldn't write anything.)

> Maybe it will be able to have more values later.
That wouldn't make any sense. Currently he uses xx_num_contents (and not some attribute in xx_el or xx_change) because its value is always constant. If it could have different values (e.g. if a changepage could have multiple extended change targets???), he'd use a real attribute in the element change info, something like &xx_change.num_target_contents.
I don't have to add much here -- this is all perfectly right and a very precise description how things work here!

Just some comments and changes I just made to the code:

In setElementChangeInfoToDefaults():

Looks like this now:

Code: Select all

#if 0
  /* (not needed; set by setConfigToDefaultsFromConfigList()) */
  xx_num_contents = 1;
#endif
This part was indeed not needed at all, as it gets automatically initialized to the default value in the function setConfigToDefaultsFromConfigList() that directly follows these lines.

In SaveLevel_CUSX():

Looks like this now:

Code: Select all

#if 0
  /* set (fixed) number of content areas (may be wrong by broken level file) */
  /* (this is now directly corrected for broken level files after loading) */
  xx_num_contents = 1;
#endif
This part of the code was really needed -- why? See Tomi's explanation above! This could only happen by reading a broken level file (not written by R'n'D, but maybe by a buggy external tool -- I try to be as save and reliable as possible when reading external data).

For the rare case of a broken level file that really could set xx_num_contents to zero, I've added the following lines to LoadLevel_MicroChunk() -- that's where handling this rare case really belongs to (and not later when trying to write the level again).

Looks like this now:

Code: Select all

        if (num_entities > max_num_entities)
        {
          Error(ERR_WARN,
                "truncating number of entities for element %d from %d to %d",
                element, num_entities, max_num_entities);

          num_entities = max_num_entities;
        }

        if (num_entities == 0 && (data_type == TYPE_ELEMENT_LIST ||
                                  data_type == TYPE_CONTENT_LIST))
        {
          /* for element and content lists, zero entities are not allowed */
          Error(ERR_WARN, "found empty list of entities for element %d",
                element);

          /* do not set "num_entities" here to prevent reading behind buffer */

          *(int *)(conf[i].num_entities) = 1;   /* at least one is required */
        }
        else
        {
          *(int *)(conf[i].num_entities) = num_entities;
        }
(The first "if" block was just added to see where the second, new block belongs to.)

That way, the code should be more clean now (and also more save, as the rare bug now gets corrected where it could appear, and not somewhere at a later point in the code).

Thanks a lot, Tomi, for bringing up this very good point(s) and doing the anaylsis right away! The code should now be a bit better than before! :-)
Zomis
Posts: 1502
Joined: Mon Jun 21, 2004 1:27 pm
Location: Sweden
Contact:

Post by Zomis »

Holger wrote:> I wonder if there's a way to make a distinction between designtime
> RND-variables (those used when creating levels) and runtime variables
> (those use for playing levels, playing around in the main menu, etc...)

Oh well, currently there's a lot in src/main.h, so it's sometimes hard to tell (unless there are comments which directly tell what's good for what, and where). I already tried putting some things to src/game.h etc. ...

> So is there a way to know which ones of the variables and types are
> designtime?

For now, you have to "grep" for the types/values in question... A good rule of thumb is: If you find it in src/files.c, it's most probably a "design time value" (like level data), while stuff found in src/game.c is most probably runtime only.
Yes indeed it's a lot, but I think I've extracted the most important parts now.
I think I've never used any game.c/h stuff at all for my utility-programs, just watching the size of game.c makes me feel sick ;)
Post Reply