nextonphotos commented 2 years ago

Hi! I've recently been encountering an issue that causes a game crash and seems to be related to unloading zones.

I'm in the early stages of developing a game and have implemented your zone loading system. It works great when the player walks between zones, but I also have a teleport system. If I teleport from a zone with no loaded neighbors, it works as expected. When I teleport from a zone with loaded neighbors, however, the game crashes, often silently.

I tried to trace the problem for quite a while in my code, but couldn't find a cause other than the crash happened when the unload delay timer fired, so I went to the demo project to see if I could reproduce it there.

I was able to reproduce my issue in the demo by just teleporting the player to a position where multiple zones need to unload.

I made a little testing example by adding the following to the _input function in the world.gd file of the demo:

    if Input.is_key_pressed(KEY_0):
        player.global_position = Vector2(480,290)
    if Input.is_key_pressed(KEY_1):
        player.global_position = Vector2(2200,290)
    if Input.is_key_pressed(KEY_2):
        player.global_position = Vector2(3000,290)

Then run the game and click Demo 2D.

Pressing 0 teleports the player to roughly the starting position.
Pressing 1 teleports the player to a spot where Zone01 unloads, but the adjacent zones (2 and 4) remain loaded. This works exactly as expected.
Pressing 2 teleports the player to a spot where Zone01, Zone02, and Zone04 all should unload. This crashes.

I've been trying to figure out a solution for a while, but I'm new to Godot and am unsure how to fix this issue. Any ideas?

I can also cause a crash by teleporting back and forth between 0 and 1 really fast. I don't know if this would actually be a situation a player could achieve during play, but I'd like to find a way keep that from crashing as well. It seems like perhaps the unload delay should be reset when the zone is reentered? I notice that if I teleport away from a zone, back into it, and then back out before the first timer times out, then the original zone will unload based on the first time the player left the zone, not the most recent time they left. Perhaps this is the cause of the crash?

Thanks for your help and for the great system!

Yogoda commented 2 years ago

Thank you for the report, I will check this when I have the time.

nextonphotos commented 2 years ago

I've messed around with things a bit and may have found a solution. I don't know if it's ideal, but my test cases no longer crash.

I assume the crash is happening because more than one zone is attempting to unload at the exact same time and causing some sort of collision somewhere, so I just added a simple unload queue that only unloads one zone per _process call.

I did this by changing the final for loop in _remove_zone to be

for zone_id in loaded_zones:
    if not zone_id in keep_zones:
        unload_queue.push_back([zone_id, loaded_zones[zone_id]])

and then I added

var unload_queue = []
func _process(_delta):
    if(unload_queue.size() > 0):
        var zone = unload_queue.pop_front()
        var zone_id = zone[0]
        var zone_instance = zone[1]
        _print(str("pruning: request unload ", zone_id))

        emit_signal("zone_about_to_unload", zone_id, zone_instance)

        BackgroundLoader.request_unload(zone_id)

        loaded_zones.erase(zone_id)

I haven't extensively tested this and it may not be the best solution, but the demo and my game no longer crash when teleporting via the specific conditions I mentioned in the first post and, as far as I can tell, this doesn't impact any of the other functionality. I haven't tried it at all in the 3D version, however.

This doesn't solve the possible situation when a player teleports quickly back and forth potentially unloading a zone much faster than the unload delay, since it doesn't reset the timer if you reenter a zone after it's been queued to unload, but that should be a relatively simple thing to add if the rest of the solution is sound.

What are your thoughts?

Yogoda commented 2 years ago

Hello!

Wow, that's very interesting. I am indeed able to reproduce the issue, but it is a bit random and it doesn't occur if I enable the debug.

The background loader uses a message queue, so only one message is treated at the same time. Also, only the first zone is unloaded, the two middle zones are just detached from the tree but kept in memory (only zones 2 zones away are unloaded).

I will need to look at this in detail when I am not tired. Thanks for the report!

nextonphotos commented 2 years ago

Yeah, I thought the issue was quite weird especially since it isn't consistent.

For example, I code with VS Code instead of the built in editor. If I run my game from VS Code it will work fine under certain circumstances where it crashes if I run it from Godot, then running from VS Code crashes in other circumstances.

It always seems to happen when the unload delay time hits.

I'm quite new to Godot, but let me know if there is anything I can do to help track down the issue!

Thanks!

On Wed, Feb 23, 2022, 2:46 PM Yogoda @.***> wrote:

Hello!

Wow, that's very interesting. I am indeed able to reproduce the issue, but it is a bit random and it doesn't occur if I enable the debug.

The background loader uses a message queue, so only one message is treated at the same. Also, only the first zone is unloaded, the two middle zones are just detached from the tree but kept in memory (only zones 2 zones away are unloaded).

I will need to look at this in detail when I am not tired. Thanks for the report!

— Reply to this email directly, view it on GitHub https://github.com/Yogoda/ZoneLoadingSystem/issues/16#issuecomment-1049201401, or unsubscribe https://github.com/notifications/unsubscribe-auth/AESORKRWTWDMETFU35HZN23U4VBTLANCNFSM5O7RTO6A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

Yogoda commented 2 years ago

Hello,

It seems the issue is since you teleport, the zone 1 is unloaded then it is removed from the tree, instead of the other way round usually.

This is tricky to solve, but here is a code that seems to work fine (zone_loader.gd):

remove zone from the scene tree

func detach_zone(zone_id):

var area = get_node(zone_id)
var zone = area.get_node_or_null(zone_id)

if zone:

    # warning-ignore:return_value_discarded
    get_tree().create_timer(0.0).connect("timeout", self, "remove_from_tree", [area, zone])

    _print(str("zone ", zone_id, " detached"))

func remove_from_tree(area, zone):

if is_instance_valid(zone):
    area.remove_child(zone)

Please try the modification and tell me if that fixes your issue for all cases.

Thanks, Joël

nextonphotos commented 2 years ago

Hi Joël!

Sorry for the delay. I just added your fix to my game and, so far, the crashing seems to be resolved. I haven't put it through extensive testing, but the tests I used before are working as they should.

Thanks for the fix and for the helpful system!

Yogoda / ZoneLoadingSystem

Possible zone unloading bug #16

remove zone from the scene tree