> 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! :-)