derkork / godot-statecharts

A state charts extension for Godot 4
MIT License
734 stars 35 forks source link

Added easy integration for creating in-editor project settings and toggleable normal/configuration warnings #63

Closed RogerRandomDev closed 8 months ago

RogerRandomDev commented 8 months ago

project settings can be easily added now, along with making use of two for in-editor and in-game warnings for non-intended nodes being added to the state chart. it won't prevent and the warning is disabled by default.

derkork commented 8 months ago

Thank you for submitting this PR and sorry that it took me a while to look at this. After looking at it, I wish we had discussed this before you spent the time implementing it, though.

Nesting non-statechart notes between statechart nodes is not a problem at all, the library ignores such nodes, just as every other Godot node does (e.g. you can mix control and node2d nodes, you can put non-collision shape nodes below areas, etc.). A warning should indicate a misconfiguration that tells the user "this will not work if you do it this way". Nesting unrelated nodes between the state chart nodes is no misconfiguration as far as the library is concerned - it will work just fine. Hence I think there should be no such warning in the library. This also eliminates any need for this warning to be configurable.

Maybe I'm overlooking a use case here, so if you could give me some details on why do you think this is necessary (having a real-world example would be awesome!), that would help a lot. I'm putting this on hold for now.

SirPigeonz commented 8 months ago

I also don't understand why putting other nodes next to State Charts nodes would be bad. It often could be even useful to do that, and warning would signal that it's not legal for some reason, even if it is.

RogerRandomDev commented 8 months ago

yeah there isn't really a use case for it now that i've had it. it was mainly just because i preferred having the ability to keep it integrated only within itself. (TL:DR) no use case, simply a way to encourage keeping it separate in an attempt to make the scene tree visually cleaner in my use case

derkork commented 8 months ago

Thanks a lot for the clarification! I'd rather not have these warnings it in the library though as this is more of a cross-cutting concern over all code/scenes in the project and does not really belong into the core of what this library is trying to do.

I wonder if something like this could be part of some other say "linting" add-on where the editor would give warnings to enforce a consistent style in the node placement of scenes (maybe even naming conventions). Could be useful for projects with teams, maybe even with configurable, extensible rules.