Attnam / ivan

Iter Vehemens ad Necem - a continuation of the graphical roguelike by members of http://attnam.com
GNU General Public License v2.0
297 stars 42 forks source link

Splitting a stick causes a crash. #574

Open Coolthulhu opened 4 years ago

Coolthulhu commented 4 years ago

Attempting to split a stick causes a crash with a very uninformative error message about game balance and realism that totally doesn't belong in an error message, but in a PR review process/"how to contribute" notes.

crafthandle::SpawnItem special-cases CIT_STICK, but then checks craftcore::canBeCrafted(itSpawn) and kills the game if that returns false. Sticks are not craftable, since the canBeCrafted only special cases lumps (by dynamic_cast - shouldn't there be a item::IsRawMaterial() for this? There's an IsBanana()...). Sticks, as category MISC, are not "naturally" craftable. Stones only avoid the crash by having category VALUABLE instead. Lumps are FOOD (?!), but special cased.

AquariusPower commented 4 years ago

The message about "game balance and realism", if I am not wrong, was about trying to craft anything magical like wands, scrolls or quest items, basically based on craftcore::canBeCrafted(itSpawn) results.

Originally, all lumps were only of the food kind I think, other lumps materials appeared after crafting code being: unusable (if from non-meltables) or usable (from meltables), extra work required to create another sub-session to grant food is only from edibles :>

item::IsRawMaterial(), I think this check could be used to ignore the craftcore::canBeCrafted(itSpawn) check as apparently it is in the split flow too (I am not reading the code now and this may not be the best choice, may be put it inside canBeCrafted() call ),

but we need a specific test case, because, as I remember, I could split (wooden) sticks w/o problems, what was the volume (cm3) of that specific stick and what material it was made of?

Coolthulhu commented 4 years ago

but we need a specific test case, because, as I remember, I could split (wooden) sticks w/o problems, what was the volume (cm3) of that specific stick and what material it was made of?

I'm getting the crash when trying to split balsa sticks made from smashed walls. It doesn't crash if I just press enter on the "split into how many parts?", but it seems that it just causes it to abort rather than split, since I don't end up with more sticks. I tried it on a granite stick (from a dismantled weapon), uranium stick (from dismantled worm ring) and sandstone stick (from a dismantled sandstone dagger) and all caused the same crash.

The message about "game balance and realism", if I am not wrong, was about trying to craft anything magical like wands, scrolls or quest items, basically based on craftcore::canBeCrafted(itSpawn) results.

Yeah, the actual error is essentially "One function allowed crafting, but other didn't. At the moment crafting the following is not supported: items with +/- bonus, books, food, misc items, items with postfix in name.".

AquariusPower commented 4 years ago

I always pressed ESC instead of Enter in case I give up. The Enter with empty input split value should just work as "give up", this needs fixing indeed! (Also, the input value must be a positive or negative value, as it can work as a single cut, but I think there is a check already for that)

Sticks are in what section? isn't it "misc"? if so, that could be the allowed exception we should code!

btw: items with +/- bonus (enchanted), books (no time to write them), food (didnt learn to cook, a easy way to explain why cant create cans with food mainly), misc items (generic blocker to easify coding, need allowed exceptions I guess), items with postfix in name (special uniques I guess)

Coolthulhu commented 4 years ago

I always pressed ESC instead of Enter in case I give up.

I'd expect it to default to something. The text block says "[2 or more]", which sounds like it wants to default to 2.

AquariusPower commented 4 years ago

but it can be negative, what is better? hit backspace and type a negative value or protect against empty input, because someone may hit backspace and leave it empty and hit Enter :)

also, I never used 2, only...to fight against necromancers when splitting corpses which they turn into zombies
Coolthulhu commented 4 years ago

but it can be negative, what is better?

What can be negative? Defaulting to 2 on empty string can only result in negatives if someone tries to split a bugged sol stone obsidian.

AquariusPower commented 4 years ago

no no, split can also be used to provide a single precise cut, press F1 to see the help, it uses a negative number for that.