[crossfire] safe/common item stack/inventory iteration?

[Andreas Kirschbaum]

Mark Wedel wrote:
> The only really foolproof way would be to add some flag - something
> like 'FLAG_NEEDS_TO_BE_DESTROYED'. Instead of the object actually
> getting destroyed, that flag is set, and then there is some other
> function later that goes through and cleans these objects up.
>
> However, that would add a lot of processing time.
[...]
> But because those calls are used everywhere, that cleanup routine
> would have to examine _all_ of the objects - and lots of other code
> would have to be modified to be aware of these pending objects (in the
> for loops above, it would need to skip over these, etc).

This "huge overhead" can be reduced to a (very small) constant overhead:
add all objects that get marked with FLAG_NEEDS_TO_BE_DESTROYED to a
list of objects_to_be_destroyed. After the processing is done (either
after the loop, or the tick, or periodically after X ticks) actually
delete all objects from objects_to_be_destroyed. At this time no other
processing is active, therefore nothing will break.


> In any case, what I was thinking about was more trying to make macros
> so that when we find cases of that loop needing different/better
> processing, it is just a matter of updating the macros, not of
> updating 50 different for loops in the code.

Generally speaking, I don't like such macros at all: I think they make
the code much harder to read. This especially holds for new developers.
Besides that, they might interfere with code formatters or editors with
automatic indenting.

Therefore I'd suggest to add a function (or function-like macro), say
ob_is_valid(), which might be used as follows:

| for (ob = container->inv; ob != NULL; ob = ob->next) {
|     if (!ob_is_valid(ob))
|         continue;
|     <normal processing of object>
| }

The function ob_is_valid() just checks whether
FLAG_NEEDS_TO_BE_DESTROYED is set.

IMO this solution is easier to read than hiding the loop in a macro.
Besides that, it is probably not more work to change the code this way
than to replace loop with a macro.

Note: you might want to add more such functions to check for "item"
objects (when processing player's inventories), or to check for visible
objects (when processing map tiles: skip (invisible) DMs, forces, etc.)


Andreas