apocalyptech / eschalon_utils

Eschalon Books I, II, and III Character and Map Editors
http://apocalyptech.com/eschalon/
GNU General Public License v2.0
8 stars 3 forks source link

Make sure that the "quantity" field on items isn't being silently changed #33

Closed apocalyptech closed 10 years ago

apocalyptech commented 10 years ago

This is actually part of a larger issue with how we're handing our GUI/object integration, but more functionally right now:

Awhile ago I added some code to default the "quantity" field to 1 whenever data was put into any of the other item fields. I'm pretty sure this means that if someone were to load up an item with quantity zero, the code would sort of silently change the quantity over to 1 when the page loads. In the character editor, this would at least highlight with red to alert the user to the change, but in the map editor it would be nearly entirely silent.

So the main question is really: do we care? A quantity of zero causes strange behavior in the Eschalon engine, and it's difficult to think of a scenario where a mod author would intentionally set it that way. On the other hand, I've historically tried to make sure that the editor only ever changes things explicitly changed by the user, on the theory that Humans Know Better.

If we do care, the question is how to fix it. Probably with a "populating_item_window" flag, which might already exist (I seem to recall I've done something like that on at least one dialog). On a bigger level, though (and this probably deserves its own issue), we should probably stop having such a tight GUI/object integration.

apocalyptech commented 10 years ago

After verification: we are, in fact, silently changing the quantity from 0 -> 1, of items which have values set.

Now, as I mentioned, I must decide whether or not to do anything about it!

apocalyptech commented 10 years ago

Okay, in the end I decided I did care. If the editor were to silently "fix" the quantity value while the item screen is loading, a mod author who was checking a map to see if it had gotten hit with the zero-quantity thing might see the quantity as "1" and think that their maps were okay after all. I think the pool of affected people is quite small, and probably already aware of it, but I'll err on the side of being safe. So! No transparent fixing. Closing this out!