Post by Leonais in USB - Unofficial Strive Bugfix v0.6 - v0.5.23.b

Viewing post in USB - Unofficial Strive Bugfix v0.6 - v0.5.23.b

The Gorn/Forstford mod probably won't work under latest version, I now know it has no survivability, so check the code directly.

For me, the two biggest bugs in the game are (1) the autocombat glitches in the next battle when you kill the final enemy using autocombat and (2) no save/load of guild contracts (or slaves) allows exploiting. The autocombat thing looks seriously hard to track down and it might involve UI files we can't mod. I'm assuming the save/load problems will require a undesirable change to the save file format.

There's another frequent display bug where the group panel is not updated after you buy or discard items. It could also happen at other times, such as after combat. It might be hard to pin down all the occassions but this is fixable.

ankmairdor5 years ago (2 edits)

The autoattack bug starts in "combat.gd" on line 1011 "yield(self, 'skillplayed')" in enemyturn().

Yield is a coroutine command which pauses code execution, runs the first argument(combat.gd object in this case, not sure, but probably just resumes updates), and resumes when the second argument is given as a signal. The 'skillplayed' signal is given near the end of useskills(), but after checking if the player has won. Thus if the player wins during autoattack, then the current attack execution is paused until a skill(attack is a skill) finishes in the next battle. The double attack is the completion on the previous battle's unfinished attack cycle.

A proper fix would be probably be a comprehensive re-write, but a quick fix is possible. First, add "if period != 'enemyturn':" before "endcombatcheck()" in useskills(), line 917, and add a tab before the next 5 lines. Second, after the yield add "if endcombatcheck() != 'continue':" with "break" on next line (with one more tab before it). Though this change allows updates once more for any combatants still in fight. Adding the complete 5 line endcombatcheck from useskills() before "for i in enemygroup + playergroup:"(just below the yield), with "return" after "playerwin()" should prevent the additional update. I've verified that this works for some basic combat.

ProofByContradiction5 years ago

The autocombat glitch may be partially my fault, depending on what exactly you're talking about. I spent a long time stepping through memory dumps and reverse engineering how the autocombat stuff works to finally find a race condition involving the animation handling -- ref: https://itch.io/t/300782/bug-linux-x86-64-multiple-versions-race-condition-cause...

The animation actually gets run with autocombat, but it's set to a small value. Autocombat can apply all the moves and end the battle, but since the coroutines are interdependent, on a fast machine one thread can terminate before the other is serviced; here, this meant the battle was ended and characters were removed from the arena prior to the animation completing.

My fix does allow the last move to always complete instantaneously without animation, but I wanted to change as little code as possible -- I didn't exactly have a lot to work with. I would advise extreme caution, because 'here be dragons'. Any complete fix that were to work 100% consistently is very likely going to be quite a bit more work than a check here or there: the combat loop code is kind of a mess (though it actually does approximately work) and needs to be fully decoupled from rendering. If it were chunked into an event dispatch model marshalled by the animation handler, any battle glitches would be solved.

ProofByContradiction5 years ago (1 edit)

I should clarify the intent of my post is mostly just a heads-up that addressing combat isn't going to be the easiest thing, nothing more. That said, two thumbs up for putting together fixes for other various annoyances in the game. I quite like this game and feel it's definitely worth attempting to maintain!

ankmairdor5 years ago

I've skimmed your issue, and indeed moving "emit_signal("skillplayed")" lower down seems like it would have caused this issue. But I tested reversing your fix and this autoattack bug still happened, so I think coroutine race conditions are definitely at play. I think that the reason I gave for why my fix works was incomplete. My fix works because it limits possible outcomes by using a variable which is not likely to be subject to a race condition.(accidental success)
I am uncertain, but I think that my fix will inadvertently undo your fix, so it can only be applied safely to non-linux games. As both of us have pointed out a proper fix involves a significant rewrite of combat.