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

Godot notifications changed, IAutoOn callbacks work differently #19

Open
rmvermeulen opened this issue Nov 15, 2024 · 2 comments
Open

Comments

@rmvermeulen
Copy link

Firstly, thanks for all the great work on Chickensoft's libraries and articles! I like it a lot 😄

Unless I'm overlooking something silly, it appears Godot (v4.3.stable.mono.official [77dcf97d8]) does no longer emit notifications for unimplemented methods. Which is great, in principle!
It seems the following example does not work unless I uncomment the override void _PhysicProcess(double delta){}...

[Meta(typeof(IAutoNode))]
public partial class Player : CharacterBody3D
{
    // mixin contract
    public override void _Notification(int what)
    {
        GD.PrintS("Notification", what);
        this.Notify(what);
    }

    // public override void _PhysicsProcess(double delta) { }

    public void OnPhysicsProcess(double delta)
    {
        GD.Print("OnPhysicsProcess");
    }
}

Unfortunate, but I can just use the default method names.

@jolexxa
Copy link
Member

jolexxa commented Nov 15, 2024

Physics and normal processing update callbacks are expensive and unnecessary if not customized, so they are not enabled if you don't override the method the Godot source generators expect.

I simply enable the relevant updates in the game demo: https://github.com/chickensoft-games/GameDemo/blob/f5f36ff3d6cb5c1a65c2b9ad91933dc5f923df49/src/player/Player.cs#L146

I need to get this documented in the AutoInject readme because this comes up pretty often.

@rmvermeulen
Copy link
Author

rmvermeulen commented Nov 16, 2024

Thanks, that makes total sense!

I figured it was something along those lines, but I didn't realize override _PhysicsProcess implicitly set SetPhysicsProcess(true). It's documentation does explain:

Note: If _physics_process is overridden, this will be automatically enabled before _ready is called.

I lazily assumed it was just based on the codegen. A reminder of this intended behavior in the readme would defenitely help avoid this pitfall.
EDIT: clarification

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

No branches or pull requests

2 participants