Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changing node parent produces Area2D/3D signal duplicates #14578

Open
Tracked by #45334 ...
Zylann opened this issue Dec 11, 2017 · 33 comments
Open
Tracked by #45334 ...

Changing node parent produces Area2D/3D signal duplicates #14578

Zylann opened this issue Dec 11, 2017 · 33 comments

Comments

@Zylann
Copy link
Contributor

Zylann commented Dec 11, 2017

Godot 2.1.4, Godot 3.0 beta 1
Windows 10 64 bits

I found that if you teleport-away and reparent a node which is inside an Area2D, you will get an area_exit, but ALSO extras notifications body_enter and body_exit, two in my case, even though the object is not anymore in the area.

This gave me headaches for several hours in my game until I narrow it down to this, because this behavior causes my teleport system to trigger everything multiple times, and I can't find a way to avoid this so far without atrocious hacks...
It does this wether I teleport from fixed_process or not, and emptying layer/collision masks before teleport has no effect.

Here is a demo project reproducing it:
2.1.4: TeleportArea2D.zip
3.0b1: TeleportArea2D_Godot3b1.zip

Repro:

  1. Launch main scene, notice you get one body_enter because the body overlaps with the area, wich is expected
  2. Press any key, this teleports the body and reparents it (in code, the node is removed from tree, translated, and then added to the tree)
  3. Notice you get an expected body_exit, but also two unexpected pairs of body_enter and body_exit
@Zylann Zylann changed the title Changing node parent produces Area2D.body_enter signal duplicates Changing node parent produces Area2D signal duplicates Dec 12, 2017
@Zylann
Copy link
Contributor Author

Zylann commented Dec 12, 2017

I've done some probing in Area2D, and I got the following:

-----_body_inout
Body enter
-----_body_exit_tree
Body exit
-----_body_enter_tree
Body enter
-----_body_inout
Body exit
-----_body_inout
Body enter
-----_body_inout
Body exit

Looking at the code, first thing strange thing I notice is:

Area2D stays connected to the body when it leaves the tree, I see no signal disconnection here and connections aren't one-shots:

godot/scene/2d/area_2d.cpp

Lines 134 to 148 in 13c2ff9

void Area2D::_body_exit_tree(ObjectID p_id) {
Object *obj = ObjectDB::get_instance(p_id);
Node *node = Object::cast_to<Node>(obj);
ERR_FAIL_COND(!node);
Map<ObjectID, BodyState>::Element *E = body_map.find(p_id);
ERR_FAIL_COND(!E);
ERR_FAIL_COND(!E->get().in_tree);
E->get().in_tree = false;
emit_signal(SceneStringNames::get_singleton()->body_exited, node);
for (int i = 0; i < E->get().shapes.size(); i++) {
emit_signal(SceneStringNames::get_singleton()->body_shape_exited, p_id, node, E->get().shapes[i].body_shape, E->get().shapes[i].area_shape);
}
}

Then, when the body is re-added to the tree, body_enter is emitted but there is no check wether it still overlaps with the area, but in my opinion it's wrong to not have disconnected the signals:

godot/scene/2d/area_2d.cpp

Lines 116 to 132 in 13c2ff9

void Area2D::_body_enter_tree(ObjectID p_id) {
Object *obj = ObjectDB::get_instance(p_id);
Node *node = Object::cast_to<Node>(obj);
ERR_FAIL_COND(!node);
Map<ObjectID, BodyState>::Element *E = body_map.find(p_id);
ERR_FAIL_COND(!E);
ERR_FAIL_COND(E->get().in_tree);
E->get().in_tree = true;
emit_signal(SceneStringNames::get_singleton()->body_entered, node);
for (int i = 0; i < E->get().shapes.size(); i++) {
emit_signal(SceneStringNames::get_singleton()->body_shape_entered, p_id, node, E->get().shapes[i].body_shape, E->get().shapes[i].area_shape);
}
}

The 3 next emissions in _body_inout are still a mystery to me, but I have to go to sleep for now

@ghost ghost added this to the 2.1 milestone Dec 12, 2017
@puppetmaster-
Copy link

puppetmaster- commented Dec 12, 2017

This bug/behavior has existed for a long time (03.2016). My only solution was to delete the body and re-create it. You can also try to remove all connections.

I've tried several times to explain that the node2d function
set_pos, set_rot and set_scale has problems with RigidBody2D, KinematicBody2D and Area2D.
Partly works and partly there is a strange behavior. By scale I mean flipping the body around the x/y axis.

In your case, the engine thinks that the body is still in Area2D although it was moved with set_pos to a different location.

example: try teleporting a falling rigidbody2d in Godot3

(set_scale = flipping body on x/y axis)

@Zylann
Copy link
Contributor Author

Zylann commented Dec 12, 2017

@puppetmaster- deleting the body (so the entirety of any character entering a teleport) is going to be quite annoying for me. I would like to find a solution to fix this in engine at least for teleporting because it's a PITA to deal with for something that simple... otherwise I'm not doing anything fancy with the kinematic body (just using move(), no rotation, no scale).
The problem on node side seems obvious, but if the issue also extends in the physics engine I'll need some help...

@Noshyaar note this behaviour also exists in 3.0

Edit:
I found that connections made by the area to the body in C++ seem to be completely redundant, because the physics engine already notifies the node with the monitoring callback.
But even with these removed, I still get an extra enter/exit from the Physics2DServer itself.

I tried to yield() one frame at various points of the teleport code to account for the delay the physics engine might have, but it had no effect.

I'm clueless now...

@Zylann
Copy link
Contributor Author

Zylann commented Dec 13, 2017

@puppetmaster- if you're interested, I lost some hair of trial and error in order to encapsulate a workaround for my use case, specifically for Godot 2.1.4 (it likely won't work in 3.0), maybe it can be useful to others:

# See https://github.com/godotengine/godot/issues/14578
class BodyTeleporter:
	
	signal done
	
	func exec(body, new_parent, new_local_pos):
		
		# Memorize and clear collision and layer masks,
		# so the physics server will stop detecting overlaps
		var cached_collision_mask = body.get_collision_mask()
		var cached_layer_mask = body.get_layer_mask()
		body.set_collision_mask(0)
		body.set_layer_mask(0)
		
		# Move the node first, so we get a body_exit coming FROM the Physics Server
		# (it's important because I believe Area2D itself sends body_exit when the body exits the tree)
		var gpos = new_parent.get_global_transform().xform(new_local_pos)
		body.set_global_pos(gpos)
		
		# Wait one physics step for the PhysicsServer to flush the things
		yield(body.get_tree(), "fixed_frame")
		# Wait again because in my game that wasn't enough apparently...
		yield(body.get_tree(), "fixed_frame")
		
		# Now the body is out of the area, has no contact and can't collide anything:
		# time to reparent it
		body.get_parent().remove_child(body)
		new_parent.add_child(body)
		
		# Set position locally,
		# because the global one set earlier isn't valid anymore after reparenting
		body.set_pos(new_local_pos)
		
		# Restore collision masks so the body works again
		body.set_collision_mask(cached_collision_mask)
		body.set_layer_mask(cached_layer_mask)
		
		# Emit a signal so the caller can yield until teleport is done
		emit_signal("done")

Usage:

var tp = BodyTeleporter.new()
tp.exec(character, dst_room, target_pos)
yield(tp, "done")

This is how you teleport bodies under a new parent, folks.
Fixing this would be really nice, although I understand the frame delay of the physics engine, I believe none of this workaround should be required^^"

@puppetmaster-
Copy link

@Zylann OMG!! 😮 Great workaround! Thanks for your contribution. Let me know if you need some hair, I've got enough left and I'm sure I can borrow some.

@akien-mga akien-mga modified the milestones: 2.1, 3.1 Sep 18, 2018
@akien-mga
Copy link
Member

Is this still reproducible in the current master branch?

@Zylann
Copy link
Contributor Author

Zylann commented Sep 18, 2018

It reproduces in 3.1 alpha 1.

On scene start:

Body enter

Which is expected.
Then the reparent-teleport happens, and all these signals get fired, while I expected only one Body exit:

Body exit
Body enter
Body exit
Body enter
Body exit

TeleportArea2D.zip

@Piet-G
Copy link
Contributor

Piet-G commented Oct 11, 2018

I'm pretty certain this is the same issue as #20107. When re-inserting the node the position will be still be the original position and retrigger the enter/exit signals.

@akien-mga akien-mga modified the milestones: 3.1, 3.2 Jan 19, 2019
@akien-mga akien-mga modified the milestones: 3.2, 4.0 Nov 26, 2019
@t-karcher
Copy link
Contributor

Still reproducible in 3.2:
https://www.reddit.com/r/godot/comments/g9cosc/area2d_body_entered_firing_when_it_should_not/

The enter signal is emitted immediately after re-adding the red_level scene back to the tree. When I check "body.position" in the "entered" function, it clearly shows the body is far away, so the signal should not be emitted.

@golddotasksquestions
Copy link

golddotasksquestions commented Apr 28, 2020

I've simplified my minimal project some more, replaced the green level with a button. This makes the bug even more apparent.
After the player collided the first time and the red_level scene got removed and the button enabled,
on clicking the button the fade Animation that should only be triggered if the player is colliding with the Area2D starts immediately playing:

area2d_remove_bug_simplified
Minimal project:
Area2D_enter_on_remove_bug2.zip

@RabidTunes
Copy link

I think this is still reproducible.
I have 2 Node2D with a player hanging from one of them. When player touches Area2D (a door) it is supposed to be removed from one Node2D and add it as a child to the other Node2D and also changing its position, but project crashes because this throws infinite body_entered signals

I have included a minimal project in a zip file where you can reproduce this bug. (I am aware that this is easily avoided by just changing the position of the node and not doing the reparenting stuff, but in my real project i need to do the reparenting as I have 2 different scenes below 2 different viewports and I want the player to travel from one viewport to another)
bugarea.zip

@RabidTunes
Copy link

RabidTunes commented Dec 24, 2020

Forgot to mention, I've been testing and apparently, after removing child by using the method remove_child(child_to_remove) if you put yield(get_tree(), "idle_frame") afterwards, the whole issue gets fixed, even on my other project with multiple viewports.
bugarea-fixed.zip

@AttackButton
Copy link
Contributor

AttackButton commented Jan 16, 2021

I've simplified my minimal project some more, replaced the green level with a button. This makes the bug even more apparent.
After the player collided the first time and the red_level scene got removed and the button enabled,
on clicking the button the fade Animation that should only be triggered if the player is colliding with the Area2D starts immediately playing:

area2d_remove_bug_simplified
Minimal project:
Area2D_enter_on_remove_bug2.zip

As I did comment in #45131 I've find out that using the yield with the body_exited signal right after changing the body position is the way to go to fix this or maybe putting it in the _on_Area2D_body_exited().

Solution waiting the body_exited signal:
*Edit: The better solution is connecting the body_entered with the CONNECT_DEFERRED | CONNECT_ONESHOT options every time you remove and add the child node.

red_level.gd:

func _on_Area2D_body_entered(body):
	print(body.name, " entered red Area")
	$AnimationPlayer.play("transition")


func _on_AnimationPlayer_animation_finished(_anim_name):
	print("AA")
	stage_number += 1
	$Label.text = str("stage ", stage_number)
	$player.hide()
	$player.position = Vector2(128, 256)
	yield($Area2D, "body_exited")
	call_deferred("emit_signal","signal_next_level")

main.gd:

func change_to_next_level():
	if red_level in get_children():
		$Button.disabled = false
		remove_child(red_level)
	else:
		add_child(red_level)
		red_level.get_node("player").show()
		$Button.disabled = true

@AttackButton
Copy link
Contributor

AttackButton commented Jan 16, 2021

Forgot to mention, I've been testing and apparently, after removing child by using the method remove_child(child_to_remove) if you put yield(get_tree(), "idle_frame") afterwards, the whole issue gets fixed, even on my other project with multiple viewports.
bugarea-fixed.zip

I tested your MRP, and it is entering and exiting 2 times from each function. You can test this putting an counter inside of the functions.

Like this:

extends Node2D

var num: int = 0

func _ready():
	$LEFT/Door.connect("body_entered", self, "_player_to_right")
	$RIGHT/Door.connect("body_entered", self, "_player_to_left")

func _player_to_right(player):
	num += 1
	print(num)
	player.get_parent().remove_child(player)
	yield(get_tree(), "idle_frame")
	$RIGHT.add_child(player)
	player.set_position(Vector2())
	
func _player_to_left(player):
	print(num)
	num += 1
	player.get_parent().remove_child(player)
	yield(get_tree(), "idle_frame")
	$LEFT.add_child(player)
	player.set_position(Vector2())

You can avoid this connecting using CONNECT_DEFERRED | CONNECT_ONESHOT every time you change the room. Anyway the bug will still be there in the engine side.

extends Node2D

var num = 0

func _ready():
	$LEFT/Door.connect("body_entered", self, "_player_to_right", [], CONNECT_DEFERRED | CONNECT_ONESHOT)
	$RIGHT/Door.connect("body_entered", self, "_player_to_left", [], CONNECT_DEFERRED | CONNECT_ONESHOT)

func _player_to_right(player):
	num += 1
	print("num ", num)
	player.get_parent().remove_child(player)
#	yield(get_tree(), "idle_frame")
	$RIGHT.add_child(player)
	player.set_position(Vector2())
	$LEFT/Door.connect("body_entered", self, "_player_to_right", [], CONNECT_DEFERRED | CONNECT_ONESHOT)


func _player_to_left(player):
	num += 1
	print("num ", num)
	player.get_parent().remove_child(player)
#	yield(get_tree(), "idle_frame")
	$LEFT.add_child(player)
	player.set_position(Vector2())
	$RIGHT/Door.connect("body_entered", self, "_player_to_left", [], CONNECT_DEFERRED | CONNECT_ONESHOT)

@golddotasksquestions
Copy link

I can't test this right now, but will do so later. Thank you @AttackButton!
Still very annoying having to reconnect every time as this workaround only works with ONESHOT.

knightofiam added a commit to forerunnergames/coa that referenced this issue Sep 15, 2021
Player:

  - Fix some player state machine bugs preventing player from transitioning to
    the correct state due to having multiple valid triggers, which in turn is
    caused by not being specific enough when specifying trigger conditions
    during state machine initialization. This reduces strange bugs where the
    player gets stuck in a specific state or won't take a valid action like
    reading a sign, or the player doesn't respond to a specific valid input.

  - Remove player idle back animation delay when finished reading a sign, at
    least for now. It looked a bit awkward when finished reading a sign from too
    far off center.

Editor / Scenes:

  - Simplify main scene by combining upper & lower cliffs to reduce level
    complexity and duplication by almost 50%. Merge lower cliffs into upper
    cliffs and rename upper cliffs to cliffs.

  - Restructure the ground colliders to make them more reliable & way less
    buggy.

  - Reorganize & rename many nodes in Godot editor for simplicity & readability,
    which was made possible by the merging of upper and lower cliffs.

  - Reorganize the 10 waterfall sections so that section 1 is the very top and
    section 10 is the very bottom, respectively, which is a much more intuitive
    layout.

  - Lock some nodes in the editor to prevent them from being accidentally
    modified.

Tiles:

  - Heavily rework tile intersection code in Tools.cs to improve accuracy and
    eliminate bugs. Allow units per tile aka cell size to vary from 1 to 16 for
    precise placement of tiles without affecting collision and overlap
    detection.

  - Greatly simplify player updates in Cliff.cs for detecting both cliff
    enclosing & ice intersection code.

Signals:

  - Move ground detection signal callbacks for the player from Cliffs.cs to
    Player.cs which greatly simplifies everything, including not having to add
    duplicate callbacks in both classes.

  - Block more signals while changing player's animation-based collision shape
    to prevent signal callbacks from being triggered more than they should be.
    This is an ongoing workaround for
    godotengine/godot#14578 "Changing node parent
    produces Area2D/3D signal duplicates".

  - Improve naming of some signal callback methods.

Player.cs refactoring:

  - Remove the use of delta from Player.cs, where it mainly a hack; awaiting a
    certain amount of idle frames has been more efficient.

  - Add more private convenience methods for energy meter management to simplify
    the code & increase readability.

  - Add more private convenience methods for sign reading management to simplify
    the code & increase readability.

General refactoring:

  - Move more GetNode calls into _Ready methods and assign them to fields for
    efficiency, code simplicity, readability, & less possibilities for bugs.

Tools:

  - Don't throw an exception in Tools when checking if any arrow key is pressed
    except the specified one, when the specified input is invalid; just return
    false to decrease game crashes.

Extensions:

  - Add a string extension method for allowing the use of the null coalescing
    operator to work with empty strings, which greatly simplifies
    string-handling code, e.g., when using the ternary operator to return
    strings from methods.

Cleanup:

  - Remove unused & obsolete classes.
  - Remove unused & obsolete methods.
  - Remove outdated & unnecessary comments.
  - Suppress unnecessary compiler warnings with comments for increased readability.
  - Remove obsolete compiler warning suppression comments.
  - Rename some outdated variables for increased clarity on their purpose.
  - Rename some methods for simplicity.
  - Reorganize order of some fields for increased readability>
  - Remove obsolete formatter-disabling tags for code blocks.
  - Add formatter-disabling tags where necessary.
  - Add some TODO's.
  - Improve many log messages.
  - Improve code formatting.
knightofiam added a commit to forerunnergames/coa that referenced this issue Sep 15, 2021
Player:

  - Fix some player state machine bugs preventing player from transitioning to
    the correct state due to having multiple valid triggers, which in turn is
    caused by not being specific enough when specifying trigger conditions
    during state machine initialization. This reduces strange bugs where the
    player gets stuck in a specific state or won't take a valid action like
    reading a sign, or the player doesn't respond to a specific valid input.

  - Fix tile dropdown bug that was occurring, for example, when standing
    on top of arrow signs, MoveAndSlide is detecting a collision between
    the player and sign, but the player's Area2D animation collider rect
    is not actually overlapping the sign tile, and the reported
    collision point when converted to a tile cell coordinate confirms
    this. Do not drop down when there is no actual tile being
    intersected, even if a collision is being reported. Perhaps in the
    future there could be a one tile cell margin added to detect overlap
    to account for the player being very close to the tile..

  - Remove player idle back animation delay when finished reading a sign, at
    least for now. It looked a bit awkward when finished reading a sign from too
    far off center.

Editor / Scenes:

  - Simplify main scene by combining upper & lower cliffs to reduce level
    complexity and duplication by almost 50%. Merge lower cliffs into upper
    cliffs and rename upper cliffs to cliffs.

  - Restructure the ground colliders to make them more reliable & way less
    buggy.

  - Reorganize & rename many nodes in Godot editor for simplicity & readability,
    which was made possible by the merging of upper and lower cliffs.

  - Reorganize the 10 waterfall sections so that section 1 is the very top and
    section 10 is the very bottom, respectively, which is a much more intuitive
    layout.

  - Lock some nodes in the editor to prevent them from being accidentally
    modified.

Tiles:

  - Heavily rework tile intersection code in Tools.cs to improve accuracy and
    eliminate bugs. Allow units per tile aka cell size to vary from 1 to 16 for
    precise placement of tiles without affecting collision and overlap
    detection.

  - Greatly simplify player updates in Cliff.cs for detecting both cliff
    enclosing & ice intersection code.

Signals:

  - Move ground detection signal callbacks for the player from Cliffs.cs to
    Player.cs which greatly simplifies everything, including not having to add
    duplicate callbacks in both classes.

  - Block more signals while changing player's animation-based collision shape
    to prevent signal callbacks from being triggered more than they should be.
    This is an ongoing workaround for
    godotengine/godot#14578 "Changing node parent
    produces Area2D/3D signal duplicates".

  - Don't block sign signals anymore because it causes problems with reading the
    sign because it doesn't consistently detect the player is in the sign, and
    it's working fine without the blocking.

  - Improve naming of some signal callback methods.

Player.cs refactoring:

  - Remove the use of delta from Player.cs, where it mainly a hack; awaiting a
    certain amount of idle frames has been more efficient.

  - Add more private convenience methods for energy meter management to simplify
    the code & increase readability.

  - Add more private convenience methods for sign reading management to simplify
    the code & increase readability.

  - Make fields whose value changes in async methods volatile to prevent the
    wrong value from being read, causing hard to track down bugs.

General refactoring:

  - Move more GetNode calls into _Ready methods and assign them to fields for
    efficiency, code simplicity, readability, & less possibilities for bugs.

Tools:

  - Don't throw an exception in Tools when checking if any arrow key is pressed
    except the specified one, when the specified input is invalid; just return
    false to decrease game crashes.

Extensions:

  - Add a string extension method for allowing the use of the null coalescing
    operator to work with empty strings, which greatly simplifies
    string-handling code, e.g., when using the ternary operator to return
    strings from methods.

Cleanup:

  - Remove unused & obsolete classes.
  - Remove unused & obsolete methods.
  - Remove outdated & unnecessary comments.
  - Suppress unnecessary compiler warnings with comments for increased readability.
  - Remove obsolete compiler warning suppression comments.
  - Rename some outdated variables for increased clarity on their purpose.
  - Rename some methods for simplicity.
  - Reorganize order of some fields for increased readability>
  - Remove obsolete formatter-disabling tags for code blocks.
  - Add formatter-disabling tags where necessary.
  - Add some TODO's.
  - Improve many log messages.
  - Improve code formatting.
knightofiam added a commit to forerunnergames/coa that referenced this issue Sep 15, 2021
Player:

  - Fix some player state machine bugs preventing player from transitioning to
    the correct state due to having multiple valid triggers, which in turn is
    caused by not being specific enough when specifying trigger conditions
    during state machine initialization. This reduces strange bugs where the
    player gets stuck in a specific state or won't take a valid action like
    reading a sign, or the player doesn't respond to a specific valid input.

  - Fix tile dropdown bug that was occurring, for example, when standing
    on top of arrow signs, MoveAndSlide is detecting a collision between
    the player and sign, but the player's Area2D animation collider rect
    is not actually overlapping the sign tile, and the reported
    collision point when converted to a tile cell coordinate confirms
    this. Do not drop down when there is no actual tile being
    intersected, even if a collision is being reported. Perhaps in the
    future there could be a one tile cell margin added to detect overlap
    to account for the player being very close to the tile..

  - Remove player idle back animation delay when finished reading a sign, at
    least for now. It looked a bit awkward when finished reading a sign from too
    far off center.

Editor / Scenes:

  - Simplify main scene by combining upper & lower cliffs to reduce level
    complexity and duplication by almost 50%. Merge lower cliffs into upper
    cliffs and rename upper cliffs to cliffs.

  - Restructure the ground colliders to make them more reliable & way less
    buggy.

  - Reorganize & rename many nodes in Godot editor for simplicity & readability,
    which was made possible by the merging of upper and lower cliffs.

  - Reorganize the 10 waterfall sections so that section 1 is the very top and
    section 10 is the very bottom, respectively, which is a much more intuitive
    layout.

  - Lock some nodes in the editor to prevent them from being accidentally
    modified.

Tiles:

  - Heavily rework tile intersection code in Tools.cs to improve accuracy and
    eliminate bugs. Allow units per tile aka cell size to vary from 1 to 16 for
    precise placement of tiles without affecting collision and overlap
    detection.

  - Greatly simplify player updates in Cliff.cs for detecting both cliff
    enclosing & ice intersection code.

Signals:

  - Move ground detection signal callbacks for the player from Cliffs.cs to
    Player.cs which greatly simplifies everything, including not having to add
    duplicate callbacks in both classes.

  - Block more signals while changing player's animation-based collision shape
    to prevent signal callbacks from being triggered more than they should be.
    This is an ongoing workaround for
    godotengine/godot#14578 "Changing node parent
    produces Area2D/3D signal duplicates".

  - Don't block sign signals anymore because it causes problems with reading the
    sign because it doesn't consistently detect the player is in the sign, and
    it's working fine without the blocking.

  - Improve naming of some signal callback methods.

Player.cs refactoring:

  - Remove the use of delta from Player.cs, where it mainly a hack; awaiting a
    certain amount of idle frames has been more efficient.

  - Add more private convenience methods for energy meter management to simplify
    the code & increase readability.

  - Add more private convenience methods for sign reading management to simplify
    the code & increase readability.

  - Make fields whose value changes in async methods volatile to prevent the
    wrong value from being read, causing hard to track down bugs.

General refactoring:

  - Move more GetNode calls into _Ready methods and assign them to fields for
    efficiency, code simplicity, readability, & less possibilities for bugs.

Tools:

  - Don't throw an exception in Tools when checking if any arrow key is pressed
    except the specified one, when the specified input is invalid; just return
    false to decrease game crashes.

Extensions:

  - Add a string extension method for allowing the use of the null coalescing
    operator to work with empty strings, which greatly simplifies
    string-handling code, e.g., when using the ternary operator to return
    strings from methods.

Cleanup:

  - Remove unused & obsolete classes.
  - Remove unused & obsolete methods.
  - Remove outdated & unnecessary comments.
  - Suppress unnecessary compiler warnings with comments for increased readability.
  - Remove obsolete compiler warning suppression comments.
  - Rename some outdated variables for increased clarity on their purpose.
  - Rename some methods for simplicity.
  - Reorganize order of some fields for increased readability>
  - Remove obsolete formatter-disabling tags for code blocks.
  - Add formatter-disabling tags where necessary.
  - Add some TODO's.
  - Improve many log messages.
  - Improve code formatting.
@RabidTunes
Copy link

Just tested this and it's still reproducible in Godot 3.4. Seems like it will probably be fixed in 4.0

@knightofiam I think your workaround might be the cleanest so far tbh, thanks for pointing it out!

@superlazyname
Copy link

superlazyname commented May 30, 2022

I can reproduce this in 3.3.4 in 2D.

I thought I found a workaround for it but so far the best way I've found to prevent it is to just not use the event system and just use _process(delta) with get_overlapping_bodies, but that's not ideal.

I'm going to try and think of something else, I feel like this is a pretty bad bug because IMO the docs lead you right into it https://docs.godotengine.org/en/stable/tutorials/best_practices/scene_organization.html#choosing-a-node-tree-structure

I wonder if some other workaround the docs kind of recommend against because of "bad coding practices" might also work (e.g., adding the player to the scene every time).

I'll hopefully have more ideas for this later.

knightofiam added a commit to forerunnergames/coa that referenced this issue Jul 25, 2022
Preparation:

  - Re-center camera on player when not moving. This can reveal what's
    on the other side of a ravine from a broken bridge, for example,
    instead of just empty space.

  - Create cliff floor edge tiles to allow for creation of a ravine.

  - Create the cliffs of the far side of the ravine, including trees,
    flowers, grass, snow, butterflies, & signs.

  - Move sign away from bridge area, to the left of the waterfall, so
    it's not in the way.

Implementation:

  - Create broken bridge graphic & place on near & far cliff edges.

  - Allow player to stand on & and drop down through bridge posts.

  - Add swinging bridge pieces with questionable physics on near & far
    cliff edges.

Limitations (Future Features):

  - Broken bridge cannot yet be fixed.

  - Swinging broken bridge pieces cannot yet be climbed on like a rope.

  - Ravine fall doesn't kill you yet.

Miscellaneous:

- Iterate ground areas & cliff extents by group name instead of manually
  iterating by count, which caused regressions when adding new nodes of
  these types, since they were not iterated by default. Now when new
  nodes of these types are added, they will be iterated automatically,
  as long as the nodes are added to the correct groups in the editor.

- Fix detection of when the player is entering / exiting a ground area,
  which fixes a bug where a player would be cliff hanging from the very
  top of a cliff, but when attempting to climb up to the next level of
  ground, would fall instead. This is a slightly improved workaround for
  godotengine/godot#14578.

- Improve cliff tilemaps:

  - Fix tree overlapping z-index bugs by making winter layers child
    tilemaps & using y-sort.

  - Use shared tileset resource among tree tilemaps for efficiency,
    simplicity & consistency.

  - Fix flower/player overlapping z-index bugs by organizing flowers
    into background & foreground tilemaps.

  - Change all tilemap origins back to zero for simplicity &
    consistency. Adjust individual tile positions to compensate.

  - Remove redundant ice tiles and only use ice-autotile for simplicity
    & consistency. Other ice tiles can still be accessed by disabling
    autotile.

  - Ensure all individual tileset tile z-indices are zero; change the
    z-index of the tilemap instead for clarity & simplicity..

  - Fix typo in cliff-floor tile name.
knightofiam added a commit to forerunnergames/coa that referenced this issue Jul 26, 2022
Preparation:

  - Re-center camera on player when not moving. This can reveal what's
    on the other side of a ravine from a broken bridge, for example,
    instead of just empty space.

  - Create cliff floor edge tiles to allow for creation of a ravine.

  - Create the cliffs of the far side of the ravine, including trees,
    flowers, grass, snow, butterflies, & signs.

  - Move sign away from bridge area, to the left of the waterfall, so
    it's not in the way.

Implementation:

  - Create broken bridge graphic & place on near & far cliff edges.

  - Allow player to stand on & and drop down through bridge posts.

  - Add swinging bridge pieces with questionable physics on near & far
    cliff edges.

  - Swinging bridge pieces swing randomly to simulate being blown by the
    wind occasionally, and to show the player that they move.

  - Left mouse clicking on the swinging bridge pieces forcefully moves
    them around.

Limitations (Future Features):

  - Broken bridge cannot yet be fixed.

  - Swinging broken bridge pieces cannot yet be climbed on like a rope.

  - Ravine fall doesn't kill you yet.

Miscellaneous:

- Iterate ground areas & cliff extents by group name instead of manually
  iterating by count, which caused regressions when adding new nodes of
  these types, since they were not iterated by default. Now when new
  nodes of these types are added, they will be iterated automatically,
  as long as the nodes are added to the correct groups in the editor.

- Fix detection of when the player is entering / exiting a ground area,
  which fixes a bug where a player would be cliff hanging from the very
  top of a cliff, but when attempting to climb up to the next level of
  ground, would fall instead. This is a slightly improved workaround for
  godotengine/godot#14578.

- Improve cliff tilemaps:

  - Fix tree overlapping z-index bugs by making winter layers child
    tilemaps & using y-sort.

  - Use shared tileset resource among tree tilemaps for efficiency,
    simplicity & consistency.

  - Fix flower/player overlapping z-index bugs by organizing flowers
    into background & foreground tilemaps.

  - Change all tilemap origins back to zero for simplicity &
    consistency. Adjust individual tile positions to compensate.

  - Remove redundant ice tiles and only use ice-autotile for simplicity
    & consistency. Other ice tiles can still be accessed by disabling
    autotile.

  - Ensure all individual tileset tile z-indices are zero; change the
    z-index of the tilemap instead for clarity & simplicity..

  - Fix typo in cliff-floor tile name.
knightofiam added a commit to forerunnergames/coa that referenced this issue Jul 26, 2022
Preparation:

  - Re-center camera on player when not moving. This can reveal what's
    on the other side of a ravine from a broken bridge, for example,
    instead of just empty space.

  - Create cliff floor edge tiles to allow for creation of a ravine.

  - Create the cliffs of the far side of the ravine, including trees,
    flowers, grass, snow, butterflies, & signs.

  - Move sign away from bridge area, to the left of the waterfall, so
    it's not in the way.

Implementation:

  - Create broken bridge graphic & place on near & far cliff edges.

  - Allow player to stand on & and drop down through bridge posts.

  - Add swinging bridge pieces with questionable physics on near & far
    cliff edges.

  - Swinging bridge pieces swing randomly to simulate being blown by the
    wind occasionally, and to show the player that they move.

  - Left mouse clicking on the swinging bridge pieces forcefully moves
    them around.

Limitations (Future Features):

  - Broken bridge cannot yet be fixed.

  - Swinging broken bridge pieces cannot yet be climbed on like a rope.

  - Ravine fall doesn't kill you yet.

Miscellaneous:

- Iterate ground areas & cliff extents by group name instead of manually
  iterating by count, which caused regressions when adding new nodes of
  these types, since they were not iterated by default. Now when new
  nodes of these types are added, they will be iterated automatically,
  as long as the nodes are added to the correct groups in the editor.

- Fix detection of when the player is entering / exiting a ground area,
  which fixes a bug where a player would be cliff hanging from the very
  top of a cliff, but when attempting to climb up to the next level of
  ground, would fall instead. This is a slightly improved workaround for
  godotengine/godot#14578.

- Improve cliff tilemaps:

  - Fix tree overlapping z-index bugs by making winter layers child
    tilemaps & using y-sort.

  - Use shared tileset resource among tree tilemaps for efficiency,
    simplicity & consistency.

  - Fix flower/player overlapping z-index bugs by organizing flowers
    into background & foreground tilemaps.

  - Change all tilemap origins back to zero for simplicity &
    consistency. Adjust individual tile positions to compensate.

  - Remove redundant ice tiles and only use ice-autotile for simplicity
    & consistency. Other ice tiles can still be accessed by disabling
    autotile.

  - Ensure all individual tileset tile z-indices are zero; change the
    z-index of the tilemap instead for clarity & simplicity..

  - Fix typo in cliff-floor tile name.
knightofiam added a commit to forerunnergames/coa that referenced this issue Jul 29, 2022
Preliminary features:

  - Re-center camera on player when not moving. This can reveal what's
    on the other side of a ravine from a broken bridge, for example,
    instead of just empty space.

  - Create cliff floor edge tiles to allow for creation of a ravine.

  - Create the cliffs of the far side of the ravine, including trees,
    flowers, grass, snow, butterflies, & signs.

  - Create the ravine under the broken bridge.

  - Move sign away from bridge area, to the left of the waterfall, so
    it's not in the way.

Implementation:

  - Create broken bridge graphic & place on near & far cliff edges.

  - Allow player to stand on & and drop down through bridge posts.

  - Allow butterflies to perch on top of bridge posts.

  - Add swinging bridge pieces with questionable physics on near & far
    cliff edges.

  - Swinging bridge pieces swing randomly to simulate being blown by the
    wind occasionally, and to show the player that they are dynamic.

  - Left mouse clicking on the swinging bridge pieces forcefully moves
    them around.

Limitations (Potential future features):

  - Broken bridge cannot yet be fixed.

  - Swinging broken bridge pieces cannot yet be climbed on like a rope.

  - Ravine fall doesn't kill the player yet, so player will be stuck and
    be forced to respawn.

Bug fixes:

  - Add an extra, empty texture-pixel of space at the top of the bridge
    post tiles in the tileset, so the butterflies can perch on top of
    the bridge posts - otherwise they perch "inside" the top of the bridge
    posts & disappear behind the foreground bridge posts because the
    butterflies' Z values are supposed to be behind them when flying.
    (Attempting to change the Z values dynamically when perching would
    introduce noticeable visual artifacts where the butterflies would
    appear to alternate being in front of and behind a foreground bridge
    post.)

  - Make butterflies' perching colliders larger so they don't miss their
    bridge post perches.

  - Properly scale drawable perch points for debugging clarity.

  - Properly handle horizontally / vertically flipped tiles when
    perchable. Butterflies can now perch on flipped tiles, even when the
    perchable areas are only specified for the non-flipped tile.

  - Remove magic number '16.0f' from Tools#GetTileCellAtAnyOf, and
    actually calculate local / texture tile size. This is important
    because some tiles such as bridge-post's are no longer 16x16
    texture-pixels in size.

Miscellaneous:

- Iterate ground areas & cliff extents by group name instead of manually
  iterating by count, which caused regressions when adding new nodes of
  these types, since they were not iterated by default. Now when new
  nodes of these types are added, they will be iterated automatically,
  as long as the nodes are added to the correct groups in the editor.

- Fix detection of when the player is entering / exiting a ground area,
  which fixes a bug where a player would be cliff hanging from the very
  top of a cliff, but when attempting to climb up to the next level of
  ground, would fall instead. This is a slightly improved workaround for
  godotengine/godot#14578.

- Improve cliff tilemaps:

  - Fix tree overlapping z-index bugs by making winter layers child
    tilemaps & using y-sort.

  - Use shared tileset resource among tree tilemaps for efficiency,
    simplicity & consistency.

  - Fix flower/player overlapping z-index bugs by organizing flowers
    into background & foreground tilemaps.

  - Change all tilemap origins back to zero for simplicity &
    consistency. Adjust individual tile positions to compensate.

  - Remove redundant ice tiles and only use ice-autotile for simplicity
    & consistency. Other ice tiles can still be accessed by disabling
    autotile.

  - Ensure all individual tileset tile z-indices are zero; change the
    z-index of the tilemap instead for clarity & simplicity..

  - Fix typo in cliff-floor tile name.
knightofiam added a commit to forerunnergames/coa that referenced this issue Jul 29, 2022
Preliminary features:

  - Re-center camera on player when not moving. This can reveal what's
    on the other side of a ravine from a broken bridge, for example,
    instead of just empty space.

  - Create cliff floor edge tiles to allow for creation of a ravine.

  - Create the cliffs of the far side of the ravine, including trees,
    flowers, grass, snow, butterflies, & signs.

  - Create the ravine under the broken bridge.

  - Move sign away from bridge area, to the left of the waterfall, so
    it's not in the way.

Implementation:

  - Create broken bridge graphic & place on near & far cliff edges.

  - Allow player to stand on & and drop down through bridge posts.

  - Allow butterflies to perch on top of bridge posts.

  - Add swinging bridge pieces with questionable physics on near & far
    cliff edges.

  - Swinging bridge pieces swing randomly to simulate being blown by the
    wind occasionally, and to show the player that they are dynamic.

  - Left mouse clicking on the swinging bridge pieces forcefully moves
    them around.

Limitations (Potential future features):

  - Broken bridge cannot yet be fixed.

  - Swinging broken bridge pieces cannot yet be climbed on like a rope.

  - Ravine fall doesn't kill the player yet, so player will be stuck and
    be forced to respawn.

Bug fixes:

  - Add an extra, empty texture-pixel of space at the top of the bridge
    post tiles in the tileset, so the butterflies can perch on top of
    the bridge posts - otherwise they perch "inside" the top of the bridge
    posts & disappear behind the foreground bridge posts because the
    butterflies' Z values are supposed to be behind them when flying.
    (Attempting to change the Z values dynamically when perching would
    introduce noticeable visual artifacts where the butterflies would
    appear to alternate being in front of and behind a foreground bridge
    post.)

  - Make butterflies' perching colliders larger so they don't miss their
    bridge post perches.

  - Properly scale drawable perch points for debugging clarity.

  - Properly handle horizontally / vertically flipped tiles when
    perchable. Butterflies can now perch on flipped tiles, even when the
    perchable areas are only specified for the non-flipped tile.

  - Remove magic number '16.0f' from Tools#GetTileCellAtAnyOf, and
    actually calculate local / texture tile size. This is important
    because some tiles such as bridge-post's are no longer 16x16
    texture-pixels in size.

Miscellaneous:

- Iterate ground areas & cliff extents by group name instead of manually
  iterating by count, which caused regressions when adding new nodes of
  these types, since they were not iterated by default. Now when new
  nodes of these types are added, they will be iterated automatically,
  as long as the nodes are added to the correct groups in the editor.

- Fix detection of when the player is entering / exiting a ground area,
  which fixes a bug where a player would be cliff hanging from the very
  top of a cliff, but when attempting to climb up to the next level of
  ground, would fall instead. This is a slightly improved workaround for
  godotengine/godot#14578.

- Improve cliff tilemaps:

  - Fix tree overlapping z-index bugs by making winter layers child
    tilemaps & using y-sort.

  - Use shared tileset resource among tree tilemaps for efficiency,
    simplicity & consistency.

  - Fix flower/player overlapping z-index bugs by organizing flowers
    into background & foreground tilemaps.

  - Change all tilemap origins back to zero for simplicity &
    consistency. Adjust individual tile positions to compensate.

  - Remove redundant ice tiles and only use ice-autotile for simplicity
    & consistency. Other ice tiles can still be accessed by disabling
    autotile.

  - Ensure all individual tileset tile z-indices are zero; change the
    z-index of the tilemap instead for clarity & simplicity..

  - Fix typo in cliff-floor tile name.
knightofiam added a commit to forerunnergames/coa that referenced this issue Jul 29, 2022
Preliminary features:

  - Re-center camera on player when not moving. This can reveal what's
    on the other side of a ravine from a broken bridge, for example,
    instead of just empty space.

  - Create cliff floor edge tiles to allow for creation of a ravine.

  - Create the cliffs of the far side of the ravine, including trees,
    flowers, grass, snow, butterflies, & signs.

  - Create the ravine under the broken bridge.

  - Move sign away from bridge area, to the left of the waterfall, so
    it's not in the way.

Implementation:

  - Create broken bridge graphic & place on near & far cliff edges.

  - Allow player to stand on & and drop down through bridge posts.

  - Allow butterflies to perch on top of bridge posts.

  - Add swinging bridge pieces with questionable physics on near & far
    cliff edges.

  - Swinging bridge pieces swing randomly to simulate being blown by the
    wind occasionally, and to show the player that they are dynamic.

  - Left mouse clicking on the swinging bridge pieces forcefully moves
    them around.

Limitations (Potential future features):

  - Broken bridge cannot yet be fixed.

  - Swinging broken bridge pieces cannot yet be climbed on like a rope.

  - Ravine fall doesn't kill the player yet, so player will be stuck and
    be forced to respawn.

Bug fixes:

  - Add an extra, empty texture-pixel of space at the top of the bridge
    post tiles in the tileset, so the butterflies can perch on top of
    the bridge posts - otherwise they perch "inside" the top of the bridge
    posts & disappear behind the foreground bridge posts because the
    butterflies' Z values are supposed to be behind them when flying.
    (Attempting to change the Z values dynamically when perching would
    introduce noticeable visual artifacts where the butterflies would
    appear to alternate being in front of and behind a foreground bridge
    post.)

  - Make butterflies' perching colliders larger so they don't miss their
    bridge post perches.

  - Properly scale drawable perch points for debugging clarity.

  - Properly handle horizontally / vertically flipped tiles when
    perchable. Butterflies can now perch on flipped tiles, even when the
    perchable areas are only specified for the non-flipped tile.

  - Remove magic number '16.0f' from Tools#GetTileCellAtAnyOf, and
    actually calculate local / texture tile size. This is important
    because some tiles such as bridge-post's are no longer 16x16
    texture-pixels in size.

Miscellaneous:

- Iterate ground areas & cliff extents by group name instead of manually
  iterating by count, which caused regressions when adding new nodes of
  these types, since they were not iterated by default. Now when new
  nodes of these types are added, they will be iterated automatically,
  as long as the nodes are added to the correct groups in the editor.

- Fix detection of when the player is entering / exiting a ground area,
  which fixes a bug where a player would be cliff hanging from the very
  top of a cliff, but when attempting to climb up to the next level of
  ground, would fall instead. This is a slightly improved workaround for
  godotengine/godot#14578.

- Improve cliff tilemaps:

  - Fix tree overlapping z-index bugs by making winter layers child
    tilemaps & using y-sort.

  - Use shared tileset resource among tree tilemaps for efficiency,
    simplicity & consistency.

  - Fix flower/player overlapping z-index bugs by organizing flowers
    into background & foreground tilemaps.

  - Change all tilemap origins back to zero for simplicity &
    consistency. Adjust individual tile positions to compensate.

  - Remove redundant ice tiles and only use ice-autotile for simplicity
    & consistency. Other ice tiles can still be accessed by disabling
    autotile.

  - Ensure all individual tileset tile z-indices are zero; change the
    z-index of the tilemap instead for clarity & simplicity..

  - Fix typo in cliff-floor tile name.
@Dante3085
Copy link

Dante3085 commented Aug 3, 2022

Pretty sure I am battling this issue in 3.4.5.stable right now. Here is a minimal reproduction project (Move the capsule with left and right arrow keys): area2d_body_entered_triggered_multiple_times.zip

using call_deferred() for the add_child() seems to fix this for me, but when I exit the game I get errors that I leaked memory:
image

@Anutrix
Copy link
Contributor

Anutrix commented Oct 7, 2022

Is this reproducible in the current master branch?

@superlazyname
Copy link

superlazyname commented Oct 9, 2022

@Anutrix Copy/paste of what I wrote in #61584 (comment)

As of 880a017 (the latest from master as of right now) it looks like it's behaving better.
When I hit 3 from position B I am not getting that body_entered event anymore, which is great!

It seems like the body_entered issue has improved in the last 4 months.

@jruiz94
Copy link

jruiz94 commented Oct 9, 2022

I tested this against master branch.

I downloaded the bugarea.zip and converted it to Godot 4 (had to fix some collision layers because it didn't collide with the door) then tested it and it seems to keep happening, signal is fired an indefinite amount of times making the game freeze. I haven't tried the workarounds though.

That's for the 2D version, the 3D version (bugarea3d.zip) directly closes the game upon opening it without moving so I assume something in the automatic conversion to Godot 4 went wrong, but I don't know how to debug that.

@M5tern
Copy link

M5tern commented Nov 28, 2023

This is still a problem in 4. Reparenting a node while inside an area2d triggers an additional body_entered signal.

@jakofranko
Copy link

Can confirm, still seeing this in v4.1.3

@YuriSizov
Copy link
Contributor

Is this reproduceable with 4.3 dev1? https://godotengine.org/article/dev-snapshot-godot-4-3-dev-1/

@jruiz94
Copy link

jruiz94 commented Jan 8, 2024

Yes I can still reproduce it in 4.3 dev1. I quickly assembled a project to test it. I included 2D and 3D examples.

I'll leave a zip attached in this comment. If you want to test it, please run (using F6) the world.tscn scene for each example (I didn't set a main scene sorry)

bugarea-4.3-dev1-2D-and-3D.zip

@dalexeev

This comment was marked as off-topic.

@Saadies
Copy link

Saadies commented Apr 2, 2024

I encountered this problem with the Object Pooling I implemented. I have a kill box where I remove my Node, when i reenter the Node into my scene tree it triggered the on body entered no matter how and where I changed the position or global position.
My workaround looks like this:

func _enter_tree():
	if isReady:
		respawn()

func respawn():
	Shape.disabled = true
	await get_tree().physics_frame
	await get_tree().physics_frame
	Shape.disabled = false

@MajorGonzo
Copy link

MajorGonzo commented Apr 30, 2024

I had the same issue with double taps of my collider, and also had the issue of having to use call_deferred to reparent. Searching lead to this solution type which handled both problems (node type is Area2D):

    func picked_up(player: Player) -> void:
	  _being_carried = true
	  #reparent(player) # demanded call_deferred
	  monitoring = false
	  await change_parent(player)
	  monitoring = true
	  position.y -= 0.1
  
    func change_parent(player: Player) -> void:
	  await call_deferred("reparent",player, true)

@jam94mac
Copy link

jam94mac commented Nov 5, 2024

Hi, I am having an issue with this bug. Specifically with mouse_enter calls from Control Nodes. I have a playing card that has a control node that is being used to detect and call _on_mouse_entered(), and in the call I am calling reparent(). Problematically the reparent() is causing _on_mouse_exited to be called and on_mouse_entered to be called a second time, even though the mouse remains within the bounds of the control node. If reparent is called on mouse_exit call, the two fire indefinitely, without moving or removing the mouse from the card.

I have tried the await call_deferred("reparent") that is mentioned by MajorGonzo, and it does not fix anything.

Does anyone have a work around they could suggest? Thank you!

simple example of what i have written:

func _on_card_back_mouse_entered() -> void:
      reparent(highlightLevel,true)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests