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

Object::notification() doesn't fully respect its p_reversed parameter #52325

Closed
kleonc opened this issue Sep 1, 2021 · 0 comments · Fixed by #78634
Closed

Object::notification() doesn't fully respect its p_reversed parameter #52325

kleonc opened this issue Sep 1, 2021 · 0 comments · Fixed by #78634
Milestone

Comments

@kleonc
Copy link
Member

kleonc commented Sep 1, 2021

Godot version

3.3.3.stable, 4.0.dev (c97afc0)

System information

N/A

Issue description

Object::notification() always calls _notification() in order:

  1. For built-in classes - possibly in reversed order according to p_reversed parameter.
  2. For scripts - always in the same order.
  3. For extensions (4.0 only) - always in the same order.

Relevant source code:

void Object::notification(int p_notification, bool p_reversed) {
_notificationv(p_notification, p_reversed);
if (script_instance) {
script_instance->notification(p_notification);
}
if (_extension && _extension->notification) {
_extension->notification(_extension_instance, p_notification);
}
}

So for example for an object b of type B with inheritance:
Object < Node < A (gdscript) < B (gdscript)
here's a comparison of the current behavior and what I'd say is expected behavior:

call current call order expected call order
b.notification(12345, false) 1. Object::_notification
2. Node::_notification
3. B::_notification
4. A::_notification
1. Object::_notification
2. Node::_notification
3. A::_notification
4. B::_notification
b.notification(12345, true) 1. Node::_notification
2. Object::_notification
3. B::_notification
4. A::_notification
1. B::_notification
2. A::_notification
3. Node::_notification
4. Object::_notification

Also current notification()'s description in the docs is misleading because reversed parameter currently doesn't affect the order of execution of in-script _notification() overrides.

Steps to reproduce

Run such script:

tool
extends EditorScript

func _run() -> void:
	var b := B.new()
	for reversed in [ false, true ]:
		print("reversed = %s" % [ reversed ])
		b.notification(12345, reversed)

class A:
	func _notification(what: int) -> void:
		if what == 12345:
			print("A")
		
class B extends A:
	func _notification(what: int) -> void:
		if what == 12345:
			print("B")

Output:

reversed = False
B
A
reversed = True
B
A

Minimal reproduction project

Just run script above.

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

Successfully merging a pull request may close this issue.

3 participants