search

godot-extended-libraries / godot-next

Godot Node Extensions - Basic Node Extensions for Godot Engine
MIT License
957 stars 61 forks source link

Add MenuManager class #83

Closed nonunknown closed 3 years ago

willnationsdev commented 3 years ago

@nonunknown Okay, this is looking pretty good. Sorry it took me so long to give this the light of day. Just a few quick things.

  1. can you either remove the print statement or make it an optional setting? The default behavior of a node shouldn't print a message without users being able to turn it off.
  2. can you make the deferred calls optional, so that the target node isn't required to implement each of the methods? Just preface each call with an if target.has_method(...):

Once those are resolved, I'll merge it.

nonunknown commented 3 years ago

@nonunknown Okay, this is looking pretty good. Sorry it took me so long to give this the light of day. Just a few quick things.

  1. can you either remove the print statement or make it an optional setting? The default behavior of a node shouldn't print a message without users being able to turn it off.
  2. can you make the deferred calls optional, so that the target node isn't required to implement each of the methods? Just preface each call with an if target.has_method(...):

Once those are resolved, I'll merge it.

1 - about the first one yeah, I can do 2 - the defered one doesnt need to check if the function exists, if it doesnt exists, the engine doesnt call, neither throw any message

willnationsdev commented 3 years ago

@nonunknown

2 - the defered one doesnt need to check if the function exists, if it doesnt exists, the engine doesnt call, neither throw any message

Ah, gotcha. Sounds good. Can you remove the extra spacing in your script so that the file format check passes?