bug: objects don't stack

[fph]

I'm experiencing an odd bug in the development versions - after updating to the last one, objects I find do not stack anymore with those I had before the switch. I'm now carrying two separate packs of CLW and CSW, for instance. Could it be due to the recent changes in object descriptions?

[d_m]

Originally Posted by fph

I'm experiencing an odd bug in the development versions - after updating to the last one, objects I find do not stack anymore with those I had before the switch. I'm now carrying two separate packs of CLW and CSW, for instance. Could it be due to the recent changes in object descriptions?

I bet that once you drink all the "old" potions this won't happen.

We've seen these kinds of bugs with old savefiles while developing. I would recommend that people finish their games before trying the newest nightly, although (hopefully) other than minor stacking bugs like this they should be compatible.

Thanks for reporting this! Feel free to open a bug, but if this only happens with an old savefile then I expect it will be lower priority than more serious bugs.

[PowerDiver]

Originally Posted by fph

I'm experiencing an odd bug in the development versions - after updating to the last one, objects I find do not stack anymore with those I had before the switch. I'm now carrying two separate packs of CLW and CSW, for instance. Could it be due to the recent changes in object descriptions?

The effects have changed. It is possible that your old potions have the old effects and the new potions have the new effects and so they are in fact different despite having the same name. Of course, I'm just guessing.

[edit] Never mind. The screen dump in the other thread shows restore life not stacking, and that hasn't changed to my knowledge.

[fph]

Originally Posted by d_m

We've seen these kinds of bugs with old savefiles while developing. I would recommend that people finish their games before trying the newest nightly,

Thanks, lesson learned -- in the meantime, I'll have a CLW happy hour.

[scud]

It seems the problem effects *everything*.

Does anyone want to buy c.200 assorted Magic Books, seven Mushrooms of Vigor, twenty or so miscellaneous potions of a healing-related nature, and a dozen Banishments, Mass Banishments, and Destructions?

***

Quick aside: 'Vigor' is inconsistent with the English English used elsewhere.

[Magnate]

Originally Posted by PowerDiver

The effects have changed. It is possible that your old potions have the old effects and the new potions have the new effects and so they are in fact different despite having the same name. Of course, I'm just guessing.

[edit] Never mind. The screen dump in the other thread shows restore life not stacking, and that hasn't changed to my knowledge.

No, this is because the object flags for pre-existing objects are not set correctly when the savefile is loaded. If anyone can see what's wrong with rd_item_2() in load.c, I'd appreciate understanding where I messed up. As far as I can see, we give each object its ->base and ->kind flags, and that ought to make them stack with new ones, but somehow they are not getting some flags somewhere.

[PowerDiver]

Originally Posted by Magnate

No, this is because the object flags for pre-existing objects are not set correctly when the savefile is loaded. If anyone can see what's wrong with rd_item_2() in load.c, I'd appreciate understanding where I messed up. As far as I can see, we give each object its ->base and ->kind flags, and that ought to make them stack with new ones, but somehow they are not getting some flags somewhere.

Perhaps the kind flags are not yet initialized when rd_item_2 is read. I didn't bother checking.

The real problem is direct access of o_ptr->flags. Everything should go through object_flags(.,.) instead. It is bad style to copy flags from kind to object. If you folks are insistent on doing that, expect a bunch of problems both now while you work out the details and later when you're not expecting it.

I suggest you apply the attached patch at least for now. I didn't test it much, but IMO it is the right solution and any problems it causes I'd call preexisting bugs.

[PowerDiver]

I just checked the old code, and it appears that object_flags used to have the stuff I put in and it was purposely removed. That makes my suggested fix pretty pointless.

[d_m]

This isn't an area of the code I have spent a lot of time in so far.

Can you explain your reasoning? I get that you think that o_ptr->flags shouldn't be used directly because you don't want it to contain kind->flags and kind->base->flags. What's the reason for that? Like I said, I haven't messed around much with ID/item generation/etc so I don't have a lot of intuition here.

One nice thing I can see about making o_ptr->flags authoritative is that you can reduce the number of lines of code you need to write to look at object flags. That said, I am in favor of adding lots of helper methods like "obj_grants_telepathy" anyway, so maybe it's not such a big deal for me.

For instance, p_ptr->state functions this way now--it is authoritative for the player and whenever items are removed/worn or status effects start/end it is recalculated. Instead of creating player_state every time we need it, we can just use p_ptr->state.

[PowerDiver]

Hierarchies should be hierarchies. If you copy something from one level to another, you are just messing things up.

Imagine a new spell that changes a potion into a staff. If you implement hierarchically, such a thing is as simple as changing the object kind and setting the number of charges. If you throw away the hierarchy, then you can't do it without a lot of work. That's not to say anyone would ever do this. This is just an example of the power of the approach.

Magnate's current attempt at a fix in rd_item_2 will only work if you only add flags. If at some point a flag is subtracted from an object kind or base, things will stop stacking again.

Data structures change. If you access everything through functions, then changing a data structure can be restricted to a single file that is responsible for the access functions to the data structure. If you allow direct access, upgrading a data structure is a mess requiring you to look at the entire project.

You could imagine implementing a flag that is only active during nighttime. Or a curse that negates a random flag for a while. If everyone calls object_flags, you just add the code there to check the gameturn. That's much cleaner than removing and replacing flags on the item. If you allow direct access, you have to propagate the special casing throughout the code, and bugs will crop up if someone misses something.