gdquest-demos / godot-make-pro-2d-games

A-RPG demo made with Godot, MIT-licensed, from our Godot course
MIT License
1.01k stars 108 forks source link

Door in same position calls _on_body_entered() from each Scene of transition. #102

Open crazychenz opened 5 years ago

crazychenz commented 5 years ago

Using Godot 3.1.1 Stable, Windows 10, Building with GLES3 for Windows Desktop (debug builds only)

Steps To Reproduce

  1. Downloaded and imported project.
  2. Duplicated Grasslands level to "NewLevel".
  3. Added a Door from Grasslands to "NewLevel".
  4. Made the existing Door in "NewLevel" (previously the Cave Door), point back to Grasslands as an exit.
  5. Run the game.
  6. Run into the "NewLevel"
  7. Run back out to "Grasslands"
  8. Observe the game will transition to Grasslands for a quick moment and then automatically transition you to the Cave.

Work Around Overview

I still haven't pin pointed the actual bug, but I have a workaround that is acceptable for the moment.

Fundamentally, you need to disable the Player's collisions after the first door collision. This will prevent any other potential collisions from happening. Something like the following in Door.gd's _on_body_entered() call:

func _on_body_entered(body):
    if not body is PlayerController:
        return

    emit_signal("player_entered", map_path)

    # Must disable collision to prevent the collision from being
    # picked up in the following scene.
    body.disable_collisions()

Now you need to re-enabled the collisions after any other signals may occur, but before the player starts playing again. This can be done in LevelLoader.gd's change_level() call:

func change_level(scene_path):

    if map:
        map.get_ysort_node().remove_child(player)
        remove_child(map)
        map.call_deferred('free') #queue_free()

    map = load(scene_path).instance()
    add_child(map)

    map.get_ysort_node().add_child(player)
    var spawn = map.get_node("PlayerSpawningPoint")
    player.reset(spawn.global_position)

    # Player collisions were disabled in Door.gd. We must
    # wait a complete frame (after setting player to
    # spawn point) to flush all the collisions.
    yield(get_tree(), "idle_frame")

    # Now we can re-enabled Player collisions.
    player.enable_collisions()

    for monster in get_tree().get_nodes_in_group("monster"):
        monster.initialize(player)
    map.initialize()
    emit_signal("loaded", map)

And finally, it seemed like pausing the tree in Game.gd's change_level() call outside of the yield(transition) calls was causing some weird conditions, so I only pause the tree around the actual level transition:

func change_level(scene_path):

    yield(transition.fade_to_color(), "completed")
    tree.paused = true

    level_loader.change_level(scene_path)
    for door in level_loader.get_doors():
        door.connect("player_entered", self, "_on_Door_player_entered")

    tree.paused = false
    yield(transition.fade_from_color(), "completed")

And the enable_collisions() and disable_collisions() implementations in PlayerController.gd are:

func enable_collisions():
    $CollisionPolygon2D.set_deferred("disabled", false)

func disable_collisions():
    $CollisionPolygon2D.set_deferred("disabled", true)

Sorry for the code snippets. I would have done a fork and PR but I'm not convinced this fix is complete, its just a good enough work around for now.

LeTristanB commented 4 years ago

I have the same issue, however the code proposed here doesn't work for me : (

If I don't re-enable collision the map switching issue doesn't happen (but then the player has no collision lol).

Running in editor on: Godot 3.2.stable (steam version) MacOS Catalina 10.15.3

crazychenz commented 4 years ago

@LeTristanB, Sounds about right. I was shooting from the hip with that stuff. I ended up replacing all the gdquest code from my project, but I did still run into the issue where the collision event seems to be passed to a newly loaded scene. I'll have to look at the referenced code base again to see if I could actually patch it correctly for Godot 3.2. For now, the following is my new handler ... not sure if this pattern would work? (Just guessing for the time being.)

func _on_body_entered(body : Object, signame : String, tgtname : String, run_data : Dictionary = {}, park_data : Dictionary = {}):
    if body is Player:
        if park_data.has('doorexit'):
            body.set_global_position(park_data.doorexit.get_global_position())
            yield(get_tree(), "idle_frame")
        g.call_deferred(signame, tgtname, run_data, park_data)

The extra arguments (i.e. tgtname, run_data, and park_data) just have to do with managing state. You can ignore that. signame is the function that gets called in response to triggering the signal handler. (It actually ends up calling a function called "change_client_scene" which does a load() -> remove_child(old_scene) -> add_child(new_scene)). I think the net effect of what I am doing is performing a double yield. First yielding for a frame and then doing a deferred call to the actual signal handler. This double yielding seems to have done the trick in my subsequent projects. Still doesn't feel quite right from a design pattern though.

NathanLovato commented 4 years ago

Thanks for the report.

If I get it right, if you have doors in two different levels but that overlap with the player's location, and you spawn the player on that door, it triggers the two doors at once? From the way the game is coded, that's to be expected.

This project would need some serious code refactoring if we were to make it so you can use it as a template or something.

What I'd do is ensure the player doesn't spawn on a door, i.e. if it does, move it away from the door. Or have doors teleport to another location. See how we did this in the 2D platformer:

It's simple but it works well, we can teleport the player to any portal on any map

LeTristanB commented 4 years ago

if you have doors in two different levels but that overlap with the player's location, and you spawn the player on that door, it triggers the two doors at once?

Not quite. I think when a player is added (add_child) to a new map, if on the new map there is a door at the position where the player was in the previous map, the door on the new map will detect a collision, despite if the player not spawning at that door's location. The bug looks like it's in the engine, not the project logic per se.

Here's a video of the unwanted teleport in action (the player should not spawn back to grassland because the player never touches the cave door to grassland): https://media.giphy.com/media/LmNqxjAFJlbfwWFWHf/source.mov

Here's how to set it up the maps to reproduce: image image