Angband.oook.cz
Angband.oook.cz
AboutVariantsLadderForumCompetitionComicScreenshotsFunniesLinks

Go Back   Angband Forums > Angband > Vanilla

Reply
 
Thread Tools Display Modes
Old October 25, 2021, 09:31   #1
PowerWyrm
Prophet
 
PowerWyrm's Avatar
 
Join Date: Apr 2008
Posts: 2,901
PowerWyrm is on a distinguished road
Assert in object_absorb_partial()

In my variant, I go to the general store and buy stacks of 40 arrows until quiver is full then one more stack. At this point, object_absorb_partial() triggers an "assert(newsz2 < obj1->kind->base->max_stack);"

When I try the same in V, I don't get the assert (I suppose there is no combine pack in this case, so the bug is hidden). Looking at the code, when combine_pack() is called, I don't understand why it tries to combine full stacks at all, this makes no sense. Until the quiver is full, nothing happens (because there is no assert in this part of the code), although the code fuses the properties of two stacks of arrows for no reason, calling object_absorb_merge() on the two stacks that have no reason to be merged. Of course the assert triggers when the quiver is full since it now tries to merge one stack in the quiver and one stack in the pack that are both of size 40.
__________________
PWMAngband variant maintainer - check http://powerwyrm.monsite-orange.fr (or http://www.mangband.org/forum/viewforum.php?f=9) to learn more about this new variant!
PowerWyrm is offline   Reply With Quote
Old October 25, 2021, 10:03   #2
PowerWyrm
Prophet
 
PowerWyrm's Avatar
 
Join Date: Apr 2008
Posts: 2,901
PowerWyrm is on a distinguished road
I added the following check at the end of object_stackable() and it seems to fix the problem:

Code:
if (obj1->number == obj1->kind->base->max_stack) return false;
if (obj2->number == obj2->kind->base->max_stack) return false;
__________________
PWMAngband variant maintainer - check http://powerwyrm.monsite-orange.fr (or http://www.mangband.org/forum/viewforum.php?f=9) to learn more about this new variant!
PowerWyrm is offline   Reply With Quote
Old October 25, 2021, 10:11   #3
Nick
Vanilla maintainer
 
Nick's Avatar
 
Join Date: Apr 2007
Location: Canberra, Australia
Age: 56
Posts: 9,172
Donated: $60
Nick will become famous soon enoughNick will become famous soon enough
Thanks, I was already thinking there was some unnecessary combining going on.
__________________
One for the Dark Lord on his dark throne
In the Land of Mordor where the Shadows lie.
Nick is offline   Reply With Quote
Old October 25, 2021, 12:50   #4
backwardsEric
Adept
 
Join Date: Aug 2019
Posts: 219
backwardsEric is on a distinguished road
Quote:
Originally Posted by PowerWyrm View Post
I added the following check at the end of object_stackable() and it seems to fix the problem:

Code:
if (obj1->number == obj1->kind->base->max_stack) return false;
if (obj2->number == obj2->kind->base->max_stack) return false;
Some of the uses for object_stackable() in Vanilla are using it for the similarity tests and assuming it's not testing the numbers involved (in cmd-pickup.c for instance). Should there be a separate function for that (there's already object_similar(), but it calls object_stackable() and also has it's own test on the numbers)? An alternative would be to not modify object_stackable() but change the tests in obj-gear.c's inven_can_stack_partial() so they reject trying to combine two stacks already at their limits (it looks like changing the tests for "remainder > limit" to "remainder >= limit" and "remainder > obj->kind->base->max_stack" to "remainder >= obj->kind->base->max_stack" would be sufficient).

Last edited by backwardsEric; October 25, 2021 at 13:07. Reason: it's cmd-pickup not cmd_pickup and obj-gear not object-gear
backwardsEric is offline   Reply With Quote
Old October 26, 2021, 09:38   #5
PowerWyrm
Prophet
 
PowerWyrm's Avatar
 
Join Date: Apr 2008
Posts: 2,901
PowerWyrm is on a distinguished road
Quote:
Originally Posted by backwardsEric View Post
Some of the uses for object_stackable() in Vanilla are using it for the similarity tests and assuming it's not testing the numbers involved (in cmd-pickup.c for instance). Should there be a separate function for that (there's already object_similar(), but it calls object_stackable() and also has it's own test on the numbers)? An alternative would be to not modify object_stackable() but change the tests in obj-gear.c's inven_can_stack_partial() so they reject trying to combine two stacks already at their limits (it looks like changing the tests for "remainder > limit" to "remainder >= limit" and "remainder > obj->kind->base->max_stack" to "remainder >= obj->kind->base->max_stack" would be sufficient).
Yeah I saw those occurences, but I don't think it matters, since it's to test if an object can be picked up using match in inventory... but if you try to pick up an object with max stack number or if you have an object with max stack number in inventory you won't ever be able to match them.
__________________
PWMAngband variant maintainer - check http://powerwyrm.monsite-orange.fr (or http://www.mangband.org/forum/viewforum.php?f=9) to learn more about this new variant!
PowerWyrm is offline   Reply With Quote
Old October 26, 2021, 18:45   #6
backwardsEric
Adept
 
Join Date: Aug 2019
Posts: 219
backwardsEric is on a distinguished road
In Vanilla, the assert can be triggered when the quiver is full and a partial stack is bought that's similar to a full stack in the quiver. There's a proposed fix up as a pull request.

Adding some test cases to exercise combine_pack() would be useful, regardless of how this specific problem is fixed.
backwardsEric is offline   Reply With Quote
Reply


Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)
 
Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off

Forum Jump


All times are GMT +1. The time now is 13:34.


Powered by vBulletin® Version 3.8.11
Copyright ©2000 - 2021, vBulletin Solutions Inc.