itch.io

Okay, I think there are actually two distinct but related problems -- both race conditions (!) -- that are compounding and resulting in "random" crashes during battle. The good news is I *think* I've been able to solve the problem. After my changes to combat.gd, I haven't seen a single crash. That said, I haven't had nearly as much time to test as I had before.

I'll apologize in advance for any inaccuracies in what follows  I had no prior experience with godot script (I work in assembly or C/C++ usually), and the combat script is somewhat spaghettified, but I think I understand it to a moderate degree now.

Rather than processing each side fully before switching to process the other, the relevant handling is broken into a couple coroutines, which can yield until execution jumps back after some engine event occurs. The player's updates happen in the useskills() function, and the enemy's happen in enemyturn(). But there's a dependency here, because enemyturn() itself calls useskills(), possibly multiple times. This could be okay, but for this to be reliable both functions would have to be waiting on the same event. Unfortunately, there are 3-4 events involved within these functions. Since the stats logic is so deeply nested, the same pattern of yields isn't guaranteed to be consistent. Finally, on top of that, one or two of these events gets fired off in response to input or UI updates from their respective threads (full, proper threads, not coroutines). The event notification will signal the coroutine and execute other remaining handler code *while the other coroutine segment is running*. This is the race condition.

This would particularly be an issue for containers that happen to be resized ever, though all global variables would be fine. This would explain basically all the crashes I had earlier. Godot docs suggest explicit mutexes, but I instead chose to ensure better guarding around the problem areas and not let anything run until fully ready.

Fixes 1 and 2 below fix my crash.

I found a second race condition occurring at the beginning of the turn -- first time I've ever seen it not happen at the end of battle. This one's actually a race condition in the true sense -- explicit timers for damage updates are involved. Fix 3 below resolves this problem too.

This code is from 0.5.19c. I've lightly massaged this file so cited line numbers aren't exact. I'll add a .patch file with exact numbers.

combat.gd Fix 1. Guard checks & period update before jumping coroutine locations. Around line 908, turn this

        self.combatlog += '\n' + combatantdictionary(caster, target, text)
        emit_signal("skillplayed")
        endcombatcheck()
        if period == 'win':
                playerwin()
        if period == 'skilluse':
                period = 'base'

into:

        self.combatlog += '\n' + combatantdictionary(caster, target, text)
        endcombatcheck()
        if period == 'win':
                playerwin()
        if period == 'skilluse':
                period = 'base'
        emit_signal("skillplayed")

combat.gd Fix 2. Same reasoning/fix applied but for more-problematic thread notifications Around line 1230, change

func damagein():
        emit_signal("damagetrigger")
        ongoinganimation = false
        yield(get_tree(), 'idle_frame')
func tweenfinished():
        yield(get_tree(), 'idle_frame')
        emit_signal("tweenfinished")
        ongoinganimation = false

to:

func damagein():
        yield(get_tree(), 'idle_frame')
        ongoinganimation = false
        emit_signal("damagetrigger")
func tweenfinished():
        yield(get_tree(), 'idle_frame')
        ongoinganimation = false
        emit_signal("tweenfinished")

combat.gd Fix 3. Ensure delaydamage always happens immediately, still drawing slower if requested. Perform a full yield to the top modelview handler before actually performing the callback, completing the current frame first. Around line 1264.

        if instantanimation == true:
                for i in timings:
                        timings[i] = 0.05
                timings.delay2 = 0
        globals.main.sound('attack')
        ongoinganimation = true
        if combatant.group == 'enemy':
                change = -change
        tween.interpolate_property(node, "rect_position", pos, Vector2(pos.x, pos.y-change), timings.speed1, Tween.TRANS_ELASTIC, Tween.EASE_IN_OUT)
        tween.interpolate_callback(self, timings.delaydamage, 'damagein')
        tween.interpolate_property(node, "rect_position", Vector2(pos.x, pos.y-change), pos,  timings.speed2, Tween.TRANS_LINEAR, Tween.EASE_IN_OUT, timings.delay2)
        tween.interpolate_callback(self, timings.delayfinish, 'tweenfinished')
        tween.start()

becomes:

        if instantanimation == true:
                for i in timings:
                        timings[i] = 0.05
                timings.delay2 = 0
        timings.delaydamage = 0.0
        globals.main.sound('attack')
        ongoinganimation = true
        if combatant.group == 'enemy':
                change = -change
        tween.interpolate_property(node, "rect_position", pos, Vector2(pos.x, pos.y-change), timings.speed1, Tween.TRANS_ELASTIC, Tween.EASE_IN_OUT)
        tween.interpolate_deferred_callback(self, timings.delaydamage, 'damagein')
        tween.interpolate_property(node, "rect_position", Vector2(pos.x, pos.y-change), pos,  timings.speed2, Tween.TRANS_LINEAR, Tween.EASE_IN_OUT, timings.delay2)
        tween.interpolate_deferred_callback(self, timings.delayfinish, 'tweenfinished')
        tween.start()

I didn't go through and check I got each possible case. I haven't seen any problems after doing this, though. I definitely ask those interested to help test. I also welcome any comments from the author, whom I'd expect would know this code better than anyone.

Cheers.