Option to prevent Snappers from snapping to everything

mpmxyz commented 5 months ago

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

SnapTargets can filter possible Snappers by references or keywords. But the other way around you can only filter by reference. In practice that means you can't prevent a Snapper from snapping to a SnapTarget with empty whitelists unless you manually setup all valid SnapTargets ahead of time.

Describe the solution you'd like

A simple fix would add an option OnlySnapViaWhitelist or similar to Snappers that would prevent them from snapping without being explicitly whitelisted within the SnapTarget.

Describe alternatives you've considered

A more complex option would be to take a step back and identify what is actually done here: The goal is to build an n x m relation between Snappers and SnapTargets where you can check for each combination if it may snap or not. At the moment this relation is primarily constructed from the SnapTarget's side. Could it make sense to add an equivalent filter on the Snapper's side? Or would an external definition be better? (reasoning: The keywords and their whitelist are almost asset-like in that they will be shared by all instances of the object you are creating.)

Additional Context

No response

Requesters

Seth the avali made me aware of this issue while fighting a stray SnapTarget that stole a piece of his body armor.

shiftyscales commented 5 months ago

This feels like it is a bug to me? The SnapTarget has allow lists for references to snappers, and keywords- if no snapper, or keyword is specified- my expected behaviour would be that the Snapper wouldn't snap to the SnapTarget.

Interesting observations:

If either of the allow lists for snapper/snap target have an entry in the list, this behaviour does not occur.
If setting a null keyword on the Snapper this issue still occurs
If setting a null keyword on the SnapTarget this issue will not occur unless the Snapper also has a null keyword
If setting a non-null keyword, e.g. Test on the Snapper, and the Snap Target has no keywords in its list, it will accept the Test Snapper despite not sharing a keyword with it.

https://github.com/Yellow-Dog-Man/Resonite-Issues/assets/54213390/16a7d495-0dc2-440b-b926-aa83028cc104

So it seems like unless the allow lists are populated, a snap target will accept any snapper, even if that snapper has a keyword specified so long as the snap target has no explicit snapper or keywords specified.

Seeking feedback from @ProbablePrime.

Banane9 commented 5 months ago

So it seems like unless the allow lists are populated, a snap target will accept any snapper, even if that snapper has a keyword specified so long as the snap target has no explicit snapper or keywords specified.

From the SnapTarget's side of things, I wouldn't call this a bug per se - otherwise a SnapTarget with no allowed items would basically be useless and you couldn't make a "wildcard" target that accepts anything.

If items and targets are properly set up, i.e. with keyword(s) on both sides, this is no problem, so I would argue that this is to be blamed on the person that (perhaps inadvertently) created a wildcard SnapTarget.

shiftyscales commented 5 months ago

otherwise a SnapTarget with no allowed items would basically be useless

Yes. That would be my expected behaviour. Otherwise it's not an 'allow list'. If that list is empty- there's nothing that's allowed- so why would the default behaviour be to allow absolutely everything?

Also as mentioned in my prior post- even if a Snapper does have a keyword specified, the SnapTarget with no keywords in its list will still accept it anyway- so even when the objects are configured properly- those SnapTargets are still 'universal' when they shouldn't be. I can't see this as anything but a bug, @Banane9.

Banane9 commented 5 months ago

Yes. That would be my expected behaviour. Otherwise it's not an 'allow list'. If that list is empty- there's nothing that's allowed- so why would the default behaviour be to allow absolutely everything?

It works just the same way the Allowed Users list on the Grabbable component works: if it's empty, anyone can potentially grab it, otherwise only the users on the list can ever do so.

Also as mentioned in my prior post- even if a Snapper does have a keyword specified, the SnapTarget with no keywords in its list will still accept it anyway- so even when the objects are configured properly- those SnapTargets are still 'universal' when they shouldn't be.

Having a SnapTarget that accepts any Snapper is useful to have, e.g. for something akin to a grabbable receiver surface - if you don't want it to be universal, you have to configure what things it should accept.

ProbablePrime commented 5 months ago

My time is limited at the moment, but I wanted to give you both a pointer.

Take a look into how the Material Tip works, That uses an empty list for snapping. That's why it exists. We could add another option or something. But this is as far as my brain will allow today.

Yellow-Dog-Man / Resonite-Issues