Yellow-Dog-Man / Resonite-Issues

Issue repository for Resonite.
https://resonite.com
138 stars 2 forks source link

A Flux node to get the Active state of itself #804

Open shadowpanther opened 11 months ago

shadowpanther commented 11 months ago

Is your feature request related to a problem? Please describe.

I'm always frustrated when ProtoFlux scripts continue to run under disabled hierarchy. While this in itself is understandable - ProtoFlux runs separately from the slot hierarchy - making a proper "disabling" check in the script to remove this behaviour when needed requires you to get a reference of something close to the script itself, for example the packing root, and check the Get Slot Active of that. This could lead to errors if you pack the script under a different root and forget to change the reference.

Describe the solution you'd like

A new Flux node that returns the Active state similar to the Get Slot Active (taking in account the active state of all parents), but taking a slot of the node itself as a reference slot. Name would probably be Get Node Active Self, Get Component Enabled Self or Is Active Self (because Get Slot Active Self is already taken).

Describe alternatives you've considered

Continue to use a reference to something to check whether the script should be disabled.

Additional Context

https://discord.com/channels/1040316820650991766/1178350028700536832

TisFoolish commented 11 months ago

This is similar to https://github.com/Yellow-Dog-Man/Resonite-Issues/issues/754, so I wonder if it'd be better to just change Get Slot Active, Get Slot, Get Parent Slot, etc to self reference when there are no input attached. I can see someone saying that it might break some flux, but I can't think of any situation where someone would use these nodes without an attached input as they'd just always be null.

Frooxius commented 11 months ago

This sounds like there's a deeper issue - ProtoFlux shouldn't run in a disabled hierarchy, if the source of the updates is itself disabled - that sounds like there's something wrong.

Would you be able to provide examples of which nodes run even when disabled?

For the issue itself, it's a bit tricky in some ways. With how ProtoFlux works, there will be cases in the future where it might not exist in the hierarchy at all - e.g. when the setup gets packed into a static asset (once this is implemented). We could provide the state of the instance that references this, but it might cause some weirdness (e.g. if you structure your node hierarchy so only some parts are disabled), so I'm a bit wary of having developers rely on this to determine if the code should run or not, although it might be the same with the native mechanism that disables things.

I think we could add something like this, which gives you the active state of whatever the "proxy" of that node is in the world.

However I'd prefer to focus more on making sure that the nodes themselves actually follow this behavior overall, rather than having users check this manually.

shadowpanther commented 11 months ago

From what I've seen, all impulse sources work when disabled, including Fire On ... nodes. And that means that a lot of computational nodes are also working when disabled.

Something like Nearest User Head - Less Than - Fire On True doesn't respect disabled state, for example. And all the impulse chain that follows that, including slot manipulation, like duplicating or deleting. It is possible to "eat" a disabled template of an edible if you know its position.

The fruit plate in the default Cloud Home has a check in the script on each fruit to circumvent the issue.

This appears to be a general issue with flux, not a separate node issue.

ohzee00 commented 11 months ago

Would it make sense for any behavior that affects the datamodel directly to be disabled? Say, driving slot transforms and such being "frozen" when the protoflux is disabled? However anything that is separate from the datamodel, ex: a elapsed time node doing it thing wouldn't be affected.

It's a interesting issue that I can't really think of a solution fully, even my example would probably be a pain to implement and/or bring inconsistent behavior.

Frooxius commented 10 months ago

Generally any nodes that are sources of updates, should stop running those updates when they are disabled. If they do not, that's an oversight - we'll need to address this.

It doesn't happen automatically with ProtoFlux, because it does exist outside - the active status should be proxied over.

Would you be able to compile a list of the problematic nodes and put them into a new issue please? I'd like to go through them and sort that out first, because I think that should be a better solution for this overall.

Zyzyl commented 10 months ago

Thanks for the input @Frooxius! To make things 100% clear for people who would like to help on a potential new issue, how can we recognise nodes which are sources of updates? i.e. what criteria defines a node which is a source vs. one which isnt

shiftyscales commented 10 months ago

Updated the issue title to reflect the underlying issue.

To my understanding @Zyzyl - an update source would just be any node that can change/propagate that change downstream.

Banane9 commented 10 months ago

Name would probably be Get Node Active Self, Get Component Enabled Self or Is Active Self (because Get Slot Active Self is already taken).

The Self in the Slot version specifically refers to checking that specific slot's active state, regardless of being in a disabled hierarchy - so including Self in the name would lead to confusion, I think. Having such a node to check if an item is active at all seems useful regardless of the underlying issue of updating nodes.

shadowpanther commented 10 months ago

an update source would just be any node that can change/propagate that change downstream.

Also any impulse-generating node - Update, Timer, Fire On .../Fire While ..., Button Events, etc. Because these impulses would likely lead to something updating - writing a value, changing transforms, creating/deleting slots. And processing an impulse itself, even if it doesn't cause any change, take at least some resources.

And anything that drives values - obvious one would be a Driver (output) node, but also the Tween and Field/Object Hook nodes that hook the drive on impulse.

Another set of nodes that probably should not update their state are Constant Lerp and Smooth Lerp, even if these are not connected to some output

The Self in the Slot version specifically refers to checking that specific slot's active state

Yeah, I know. It's just the discrepancy between Get Active User Self (Self refers to the flux node) and Get Slot Active Self (Self refers to the active state of the specified slot itself, not its hierarchy) that always bothers me :)

Frooxius commented 10 months ago

Let's not change this issue please, it'll make it more confusing and harder to work with.

I requested this gets put into a new issue in my post above. This is to keep it focused and organized, rather than changing what the issue is about mid-way.

It also does not invalidate this issue request - it can still be useful feature in any way, but I want to make sure it's not being used to work around a different bug, that we should address directly first.