Monster::transcribe_monster_actions() does not behave as expected

[Lyle-Tafoya]

Basically there are 3 closely related problems, but a little bit of explanation is required before we can get to those.

Monsters with '?' in their meleestr are expected to cheat when it comes to their combat actions. In transcribe_monster_actions() it tallies up the total number of times the player blocks and attacks each location (high, center, or low) using 2 different int arrays p_blocks and p_attacks. Whichever location the player blocks the least is where a monster with "A?" in it's meleestr will attack (assuming it passes it's check. Otherwise it attacks a random location). It stands to reason that we should also expect "B?" to result in the monster blocking whichever location the player attacks the most. This leads us to the 2 bugs:

1. Instead of blocking the location the player attacks the most, it actually blocks the location the player attacks the least. I believe this is the result of a simple copy/paste error by a previous dev for the project (possibly Laurence Brothers himself). Simply flipping from < to > in the relevant if-statements solves this.
2. The game fails to accurately tally up the number of player attacks for each location. The reason it fails is because it only looks for 'A' or 'L' in Player.meleestr. However, as far as I know, 'A' will never be found in Player.meleestr. When the player sets their combat actions (in tacoptions() found in command3.cpp), 'A' gets turned into 'C' or 'T' in Player.meleestr depending on the type of weapon wielded by the player.
3. This one may not actually be a real bug, but it kinda smells like one. transcribe_monster_actions() only checks for uppercase characters in Player.meleestr. I haven't checked every location that Player.meleestr is set in the source, but I do know that in goberserk() it sets Player.meleestr to "lLlClH". However, I do think it's' possible it uses lowercase on purpose in this instance in order to avoid having "B?" monsters cheat against the player. As a result of this, the monster will always block low (when it passes it's checks) because of the order of the if-statements it uses to determine which location has the least number of incoming attacks.

[Lyle-Tafoya]

I did some digging in the source code for older versions of the game and saw that the 1st issue I mentioned has been in the game at least as far back as v0.60.2. The second issue seems to have popped up sometime between v0.60.2 and v0.71. The 3rd issue seems to have popped up between v0.71 and v0.80.1.

While looking at this, I realized another bug which has been annoying me greatly. Whenever your combat maneuvers get reset (ie. when you remove your boots of speed), it seems to leave you with no attacks. If you look at the default_maneuvers() function (which gets called whenever your combat maneuver total drops below the number of combat maneuvers in your meleestr), you will see that it does insert 'A' into Player.meleestr even though it never gets used in tacplayer(m) (where it actually parses your meleestr to see whether you hit the enemy and whatnot). tacplayer(m) only treats 'T', 'C', and 'L' as valid attack maneuvers. This means that default_maneuvers needs to be changed to use something other than 'A' for attacks. That's the easy fix, anyways. Either that or just truncate the combat maneuvers, leaving the existing player selection in for as many maneuvers as they currently have.

Personally, I think it'd be best to refactor or even rework the entire combat maneuver system.

[Lyle-Tafoya]

One more oddity I noticed is that even though tacoptions replaces 'A' with 'C' or 'T' depending on weapon type, it doesn't update your meleestr every time you change your weapon. So you might still be setup to "thrust" even after switching to a cutting weapon or vice versa. It's also possible to "lunge" with no weapon in this way even though it would normally tell you "Can't lunge without a weapon" when setting up your combat maneuvers. I believe the best way to resolve this is to have tacoptions just put 'A' in Player.meleestr and have tacplayer() check the weapon type each time you attack. This would require changes in some other places as well. initplayer() for example, sets Player.meleestr to "CCBC". This would have to change to "ACBC". This would have the side effect of fixing issue number 2 in my original comment since transcribe_monster_actions() is already expecting 'A' in Player.meleestr.

[cwc]

Thanks for looking into this Lyle! I struggle to find time for these projects nowadays, but it's good to have this info. Shoot me a PR if you have any interest in making the changes!