Borg bugs and feature requests

[agoodman]

There seems to be an inconsistency in the borg's logic for using banishment. Shouldn't the assignment at borg-fight-defend.c:2037 ("tmp_genocide_target = b_i;") be guarded by "if (b_i)"? Otherwise, a case with a rejected immediate threat and an accepted general threat will result in banishment not being cast..

Hmmm, trying to follow this code and figure out what is going on. I think you are right. I think further, this code should have its variables renamed so that it is more obvious what is going on here.

Also, the returned value when simulating (borg-fight-defend.c:2044-2049) differs from what's returned when the spell is actually cast (borg-fight-defend.c:2091) though the latter is not used by the caller. The comments by the simulated case return values are misleading: the one at borg-fight-defend.c:2047 should be something like "/* Simulation for general threat */".

Ya, I think this routine could use a good going over. I think it evolved from just genocide for immediate threats to also handling "is a threat on the level" and "is generally a threat so getting rid of it to be safe" and I am not sure it was done completely right.
BTW, I don't want to give the impression that I have anything but the highest respect for previous borg maintainers. Mistakes happen, many by me.

Edit: Noticed later that the removal of the banished monsters from the kill list (borg-fight-defend.c:2112-2131) is done even if the scroll/spell/item/staff is not used.

I think if you can't use one of the (scroll/spell/item/staff) it should have kicked out up top. If all the or's are false, something went wrong and we already announced we are genociding something.

[backwardsEric]

There seems to be an inconsistency in the borg's logic for using banishment. Shouldn't the assignment at borg-fight-defend.c:2037 ("tmp_genocide_target = b_i;") be guarded by "if (b_i)"? Otherwise, a case with a rejected immediate threat and an accepted general threat will result in banishment not being cast.

Also, the returned value when simulating (borg-fight-defend.c:2044-2049) differs from what's returned when the spell is actually cast (borg-fight-defend.c:2091) though the latter is not used by the caller. The comments by the simulated case return values are misleading: the one at borg-fight-defend.c:2047 should be something like "/* Simulation for general threat */".

Edit: Noticed later that the removal of the banished monsters from the kill list (borg-fight-defend.c:2112-2131) is done even if the scroll/spell/item/staff is not used.

[agoodman]

From lines 1948-1949, 2283, and 2297 of player-calcs.c, off-weapon extra blows contribute to the total extra blows passed to calc_blows().

okay, I added trait for "BI_EXTRA_BLOWS" which should be all the blows granted by things that aren't the weapon. I think this will work. It doesn't do the p->obj_k->modifiers stuff but, if I am reading things right, that is always just 1 if you know that rune and if you don't the borg shouldn't have put it as an item attribute.

I should double check the extra shots and extra might calcs as well. I am pretty sure they are wrong but I am not really sure how. I updated that code while still figuring out the new structures.

[backwardsEric]

If you have a blade with extra blows, then armor that gives more extra blows, does it work or is that not allowed? I will muck up my artifacts.txt to try to give armor with extra blows to see what happens.

From lines 1948-1949, 2283, and 2297 of player-calcs.c, off-weapon extra blows contribute to the total extra blows passed to calc_blows().

[agoodman]

In recent post 4.2.5 builds, there's this compiler warning on macOS:

borg/borg-trait.c:1180:9: warning: variable 'extra_blows' set but not used [-Wunused-but-set-variable]
    int extra_blows                   = 0;

I suspect that borg-trait.c should be using extra_blows at line 1892 (though it will have to avoid double counting extra blows from the equipped weapon) missing a line around 1858 to set borg.trait[BI_BLOWS] with the new value of the extra blows from the equipment.

I will have to check if having "extra blows" on a non-weapon does something. If you have a blade with extra blows, then armor that gives more extra blows, does it work or is that not allowed? I will muck up my artifacts.txt to try to give armor with extra blows to see what happens.

[backwardsEric]

In recent post 4.2.5 builds, there's this compiler warning on macOS:

borg/borg-trait.c:1180:9: warning: variable 'extra_blows' set but not used [-Wunused-but-set-variable]
    int extra_blows                   = 0;

I suspect that borg-trait.c should be using extra_blows at line 1892 (though it will have to avoid double counting extra blows from the equipped weapon) missing a line around 1858 to set borg.trait[BI_BLOWS] with the new value of the extra blows from the equipment.

[Pete Mack]

A mage cannot dig magma for some time

[agoodman]

I didn't remember that @could now dig without having to swap to the pick, but... are you telling me @spending hundred of turns to dig through a *magma* vein with a shovel of digging is normal efficiency ?

I often dig through magma with crappy weapons.. is it that Vanilla's magma is tougher than Forg's one ? Or a question of non-optimized stats in my case (chose default human warrior for the borg, far from my usual brutes)

Ya, that was caused by the RNG bug I just mentioned... If you failed once, you failed forever for repeats with the code the way it was. I just checked in a quick fix. Hopefully this will solve "repeated commands don't seem to work so well for the borg".

[Evilpotatoe]

I didn't remember that @could now dig without having to swap to the pick, but... are you telling me @spending hundred of turns to dig through a *magma* vein with a shovel of digging is normal efficiency ?

I often dig through magma with crappy weapons.. is it that Vanilla's magma is tougher than Forg's one ? Or a question of non-optimized stats in my case (chose default human warrior for the borg, far from my usual brutes)

[agoodman]

ok, its calculation of the digging chance isn't great (code needs improvement) and additionally this character is on the hairy edge of what the borg considers "acceptable chance of digging". The game calculates the odds at 192 out of 1600
okay = (chance > randint0(1600));
so it failing over and over seems long odds... if not for a bug where the borg is resetting the random seed on repeated commands! Argh! Time to fix that one.

The digging chance calculations will go on my "todo" queue.

[agoodman]

File was indeed broken.
After several reuploads, it was broken everytime... dunno why. Maybe the hyphen ?
I don't remember my filezilla breaking sent file in the past.

Anyway, looks fine in a zip (I fixed my link).

--
I downloaded a fresh Angband 4.2.5 just to test the borg, so there's nothing particular on my version
But Is that digging behavior a real new discoverey ?
It looked so "obvious" to me I thought it would be a known issue. I'm not even sure I changed one birth option.

This zipped save file works for me. With the latest code (including the fix I mentioned doing last time) I see
You tunnel into the magma vein with treasure with your swap digger. <100x>
so it -is- using the digger. I am now checking if the code has miscalculated the chance of successful digging so it is trying to dig when it shouldn't. I am not confident in the code that calculates the digging chance with swap.

As for if this digging behavior is new, I think it is. It is definitely new for the borg to take advantage of it rather than manually swapping to the digger.

[Evilpotatoe]

File was indeed broken.
After several reuploads, it was broken everytime... dunno why. Maybe the hyphen ?
I don't remember my filezilla breaking sent file in the past.

Anyway, looks fine in a zip (I fixed my link).

--
I downloaded a fresh Angband 4.2.5 just to test the borg, so there's nothing particular on my version
But Is that digging behavior a real new discoverey ?
It looked so "obvious" to me I thought it would be a known issue. I'm not even sure I changed one birth option.

[agoodman]

I'm not able to load the savefile; is there a chance that it was generated with a modded version? Looking at the code for the tunneling command, the only reason it wouldn't be able to swap for a digger in the inventory is if the currently wielded weapon or the digger has a sticky curse, but there's no weapons or diggers with sticky curses in Vanilla.

I didn't notice the save file. Ya, I am having the same issue. It looks like it is failing when loading the dungeon. I wonder if this is pre-dungeon size fix and we happen to be past that limit on this save.

That said, the problem here, IMO, is that it doesn't tell you it used the digger. So it is probably trying the digger and failing to dig. What do you think of changing the messages to say things like
You have removed the rubble with your swap digger.
when you auto-swap or
You have removed the rubble with your weapon.
when you don't? Gets slightly wordy when
You dig in the rubble {with your swap digger/with your weapon} with little effect.
but not too bad. I can put together a quick change around this.

[backwardsEric]

@backwardsEric Shouldn't it be auto-switching to use the shovel? Is there something that would prevent this?

I'm not able to load the savefile; is there a chance that it was generated with a modded version? Looking at the code for the tunneling command, the only reason it wouldn't be able to swap for a digger in the inventory is if the currently wielded weapon or the digger has a sticky curse, but there's no weapons or diggers with sticky curses in Vanilla.

[agoodman]

@backwardsEric Shouldn't it be auto-switching to use the shovel? Is there something that would prevent this?