Borg bugs and feature requests

[Nick]

If you are using configure, Makefile.std, or Makefile.osx for the build, the dependencies in Makefile.inc don't include the borg files yet (we could update Makefile.inc now for those dependencies but could also delay that till just before the 4.2.5 release) so the borg/*.o files can get out of date with respect to the headers.

I think just update straight away, I'll be doing a final rebuild of Makefile.inc before release anyway.

[backwardsEric]

I am starting to wonder if something went wrong with the build that is giving these errors. Both gdb crashes seem... odd.

If you are using configure, Makefile.std, or Makefile.osx for the build, the dependencies in Makefile.inc don't include the borg files yet (we could update Makefile.inc now for those dependencies but could also delay that till just before the 4.2.5 release) so the borg/*.o files can get out of date with respect to the headers. I know the addition BI_EMPTY in borg1.h caused some problems with the builds on the Mac until I forced a rebuild of all the borg/*.o files.

[agoodman]

... perhaps borg_think() or borg_update() need to be prepared for cases where they are called when character_dungeon is false.

I am trying to figure out what the borg should do in that case. What keypress do you give for "cave still being built"? Space? I think I have to figure out why it is in there like that. It doesn't seem like the code should be getting to inkey_hack if it is in this state.

I am starting to wonder if something went wrong with the build that is giving these errors. Both gdb crashes seem... odd.

[backwardsEric]

this one is a little confusing to me since I check square_in_bounds right before calling map_info which asserts grid.x and grid.y are good. map_info is also calling square() before this and it is working. The only thing I can think is that player->cave != cave but I don't know when those can differ.

According to the call stack there, player->cave is NULL. I would have thought the intervals when cave was not NULL and player->cave is NULL would be short and that there wouldn't be a prompt for a player command in those intervals. pre_turn_refresh() (called just before the call to textui_get_cmd() which triggered the crash) does have logic to only update the map if character_dungeon is true (character_dungeon is false in the intervals when cave and player->cave are not to be used) so perhaps borg_think() or borg_update() need to be prepared for cases where they are called when character_dungeon is false.

[backwardsEric]

Okay so further research showed that it was leather armor that didn't have known set. I think it was just filtering through items to list for the read and this was in inventory. Since it was a mage, the leather armor was either found or bought. If it was found, I can't see a path to not having know set so it must been bought so perhaps I am not creating store items correctly.
or known was overwritten, as you fear.

How did the leather armor get through to use_aux() from do_cmd_read_scroll()? The filtering on the object type (cmd_get_item() is called with tval_is_scroll()) should have excluded it before that. From what I see the borg is going through what an ordinary player would when buying which should guarantee that obj->known is not NULL (store.c:1732-1745). The initialization of store contents in borg_resurrect() looks much the same as it does in player-birth.c: calling store_reset().

[agoodman]

another reported crash
Program received signal SIGABRT, Aborted.
__pthread_kill_implementation (no_tid=0, signo=6, threadid=140185572352000) at ./nptl/pthread_kill.c:44
44 ./nptl/pthread_kill.c: No such file or directory.
(gdb) bt
#0 __pthread_kill_implementation (no_tid=0, signo=6, threadid=140185572352000) at ./nptl/pthread_kill.c:44
#1 __pthread_kill_internal (signo=6, threadid=140185572352000) at ./nptl/pthread_kill.c:78
#2 __GI___pthread_kill (threadid=140185572352000, signo=signo@entry=6) at ./nptl/pthread_kill.c:89
#3 0x00007f7f7f425476 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#4 0x00007f7f7f40b7f3 in __GI_abort () at ./stdlib/abort.c:79
#5 0x00007f7f7f40b71b in __assert_fail_base (fmt=0x7f7f7f5c0150 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x55b5156bf6d4 "c", file=0x55b5156bf539 "cave-square.c", line=883,
function=<optimized out>) at ./assert/assert.c:92
#6 0x00007f7f7f41ce96 in __GI___assert_fail (assertion=0x55b5156bf6d4 "c", file=0x55b5156bf539 "cave-square.c", line=883, function=0x55b5156bfa70 <__PRETTY_FUNCTION__.15> "square_in_bounds")
at ./assert/assert.c:101
#7 0x000055b5154d382f in square_in_bounds (c=0x0, grid=...) at cave-square.c:883
#8 0x000055b5154d3b50 in square (c=0x0, grid=...) at cave-square.c:947
#9 0x000055b5154cf858 in map_info (grid=..., g=0x7ffd300a1eb0) at cave-map.c:132
#10 0x000055b515651608 in borg_update_map () at borg/borg5.c:3242
#11 0x000055b5156547d4 in borg_update () at borg/borg5.c:4573
#12 0x000055b51569d9d4 in borg_think () at borg/borg9.c:631
#13 0x000055b5156a2977 in borg_inkey_hack (flush_first=0) at borg/borg9.c:3180
#14 0x000055b5155e106f in inkey_ex () at ui-input.c:228
#15 0x000055b5155e36ea in textui_get_command (count=0x7ffd300a2650) at ui-input.c:1759
#16 0x000055b5155ddef2 in textui_process_command () at ui-game.c:518
#17 0x000055b5155de1d6 in textui_get_cmd (context=CTX_GAME) at ui-game.c:594
#18 0x000055b5155dee33 in play_game (mode=GAME_LOAD) at ui-game.c:921
#19 0x000055b5156b9ae8 in main (argc=1, argv=0x7ffd300a2828) at main.c:540
(gdb)

this one is a little confusing to me since I check square_in_bounds right before calling map_info which asserts grid.x and grid.y are good. map_info is also calling square() before this and it is working. The only thing I can think is that player->cave != cave but I don't know when those can differ.

[agoodman]

The code that sets up the inventory items in borg9.c looks identical to that in player-birth.c. Since borg9.c calls inven_carry() on the starting items, the path through that function for a word of recall scroll should guarantee that obj->known is not NULL at that point (in that function combining should be false and in that case obj->known->grid is zeroed out). That leaves an alternate hypothesis, but more painful to debug: obj->known was overwritten between birth and when the borg tried to use the scroll.

Okay so further research showed that it was leather armor that didn't have known set. I think it was just filtering through items to list for the read and this was in inventory. Since it was a mage, the leather armor was either found or bought. If it was found, I can't see a path to not having know set so it must been bought so perhaps I am not creating store items correctly.
or known was overwritten, as you fear.

[backwardsEric]

From what I can tell this has to do with the resurrection code and I am probably not setting the known flag on all the equipment. I thought I had this fixed but apparently not.

The code that sets up the inventory items in borg9.c looks identical to that in player-birth.c. Since borg9.c calls inven_carry() on the starting items, the path through that function for a word of recall scroll should guarantee that obj->known is not NULL at that point (in that function combining should be false and in that case obj->known->grid is zeroed out). That leaves an alternate hypothesis, but more painful to debug: obj->known was overwritten between birth and when the borg tried to use the scroll.

[agoodman]

Program received signal SIGSEGV, Segmentation fault.
gear_object_for_use (p=0x556fbbfc8c40, obj=0x556fbc2b4580, num=1, message=false, none_left=0x7fffd5a66f24) at obj-gear.c:530
530 bool artifact = (obj->known->artifact != NULL);
(gdb) bt
#0 gear_object_for_use (p=0x556fbbfc8c40, obj=0x556fbc2b4580, num=1, message=false, none_left=0x7fffd5a66f24) at obj-gear.c:530
#1 0x0000556fb9e05c3a in use_aux (cmd=0x556fba0724c0 <cmd_queue+2816>, obj=0x556fbc2b4580, use=USE_SINGLE, snd=0) at cmd-obj.c:660
#2 0x0000556fb9e06052 in do_cmd_read_scroll (cmd=0x556fba0724c0 <cmd_queue+2816>) at cmd-obj.c:747
#3 0x0000556fb9e02d67 in process_command (ctx=CTX_GAME, cmd=0x556fba0724c0 <cmd_queue+2816>) at cmd-core.c:349
#4 0x0000556fb9e02e73 in cmdq_pop (c=CTX_GAME) at cmd-core.c:378
#5 0x0000556fb9e25c40 in process_player () at game-world.c:975
#6 0x0000556fb9e25fa5 in run_game_loop () at game-world.c:1085
#7 0x0000556fb9f05e38 in play_game (mode=GAME_LOAD) at ui-game.c:922
#8 0x0000556fb9fe0ae8 in main (argc=1, argv=0x7fffd5a67248) at main.c:540


From what I can tell this has to do with the resurrection code and I am probably not setting the known flag on all the equipment. I thought I had this fixed but apparently not.

[agoodman]

One place where borg_takes is cleared and the grid array is not modified is borg8.c:4484.

interesting. That code goes back as far as I have borg source for (3.4.0. borg goes back to 2.8.0 but I don't have the source for it) Easy enough to make that go through borg_delete_take and borg_delete_kill and that seems to work but I am not 100% sure that is the only place. I can add the check all the other places do as well and that big hammer fixes it. Well, I think that is the best path for now till I can track other places this might be happening.

[backwardsEric]

I think the item in the borg_takes array is zero'd when something happens that means the borg needs to forget the item... perhaps it was destroyed in a previous fireball. I thought the code that did that also removed it from the grid array. Not sure how it could be blank in borg_takes but still exist in borg_grids though other areas of the code seem to expect it to happen sometimes and just check for it

One place where borg_takes is cleared and the grid array is not modified is borg8.c:4484.

[agoodman]

From what I understood in the code, it was the object's kind stashed in the borg_takes array that was NULL (thus leading to k_ptr being NULL when it was dereferenced at borg6.c:6526. I didn't dig enough to figure out why either that element of the borg_takes array hadn't been populated appropriately or the array index stashed in ag->take was not right.

I think the item in the borg_takes array is zero'd when something happens that means the borg needs to forget the item... perhaps it was destroyed in a previous fireball. I thought the code that did that also removed it from the grid array. Not sure how it could be blank in borg_takes but still exist in borg_grids though other areas of the code seem to expect it to happen sometimes and just check for it Like if we look at borg6.c's borg_flow_take() it does
/* Skip dead objects */
if (!take->kind) continue;
I am not sure if I should just be doing the same in this spot or track down why an item would be in the grid but not in the takes. It might have to do with stacks or ignored items...

I have been wizarding up L50 mages to run them for a while and see if I can duplicate this issue but so far no luck.

[backwardsEric]

It looks like the borg was launching an attack at something it hadn't encountered yet (kind is null). I can add a check for that.

From what I understood in the code, it was the object's kind stashed in the borg_takes array that was NULL (thus leading to k_ptr being NULL when it was dereferenced at borg6.c:6526. I didn't dig enough to figure out why either that element of the borg_takes array hadn't been populated appropriately or the array index stashed in ag->take was not right.

[Estie]

There is code to change the amount of time a borg will explore levels based on the level feeling. It doesn't work great because there are so many reasons a borg might decide to hang out, scumming money, lacking certain traits, etc.

You can scum for money on a different level, with increased chance to find something good.

Whatever the reason is to be on the particular level, if the level feeling is inferior and there is no emergency, go to the next staircase and change level. Continue with whatever you were doing on the new level. I might be underestimating the difficulty of making him do this, but if its being done, it must improve the borg's performance.

[agoodman]

testing dimension door was also an issue... fixing