Add isOwner checks to hologram parenting funcs

[Derpius]

Context

• The holoParent(n, e) and holoParentAttachment(n, e, s) functions allow you to parent holograms to any entity regardless of CPPI, including other players
• This has probably not been resolved as you can usually just block holos on your client
• This however allows you to then parent a regular prop to your now parented holo, effectively bypassing the permission checks done by the prop core
  • You can use this to create simple unblockable (without cs lua) blinders and more (like parenting invis props to players' heads to prevent them from interacting with the world)

Changes

• Adds E2Lib.isOwner checks to the two functions mentioned above, throwing an error (on @strict) when the player doesn't have permissions for the target entity
  • Probably obvious to anyone familiar with the codebase, but E2Lib.isOwner will also return true if the players are CPPI friends despite the name

Impact

• Prevents players from parenting actual props to players or entities that they do not have CPPI permissions for
• This also prevents traditional blinders which just parent a holo

⬆️ Obviously if a player has given you CPPI permissions, then this is still possible. This should minimise impact to non-malicious usage of this functionality (an example raised on our server was Santa hats).

Testing

This PR is draft until we've QA'd the branch on our end

• When using holoParent(n, e)
  • I can parent a hologram to my own entities
  • I can parent a hologram to my own player entity
  • I can parent a hologram to entities owned by a player who has given me CPPI permissions
  • I can parent a hologram to a player who has given me CPPI permissions
  • I cannot parent a hologram to entities owned by players who have not given me CPPI permissions
  • I cannot parent a hologram to players who have not given me CPPI permissions
  • When I fail to parent a hologram due to permissions
    • and I'm using @strict, an error is thrown
    • and I'm not using @strict, no error is thrown but the holo is not parented
• When using holoParentAttachment(n, e, s)
  • Same test cases as above

[deltamolfar]

In my opinion this PR should not allow props to be parented to holos that are parented to non CPPI entities, rather then holos themselves, as sometimes being able to parent holograms to other players - will let you create more advanced trajectory systems, animations, etc. Thus it would be better in my eyes to make impossible to parent other then holo entities to your holograms, that has parent set as entity that doesn't fall under allowed CPPI list.

[Derpius]

will let you create more advanced trajectory systems, animations, etc.

If you need to parent this to other players, can you not use an EGP HUD with 3D trackers instead to avoid annoying whoever you're attaching holos to?

Also as I noted in the PR description, you can still parent holos if you get CPPI permissions, which seems uncontroversially better

[Derpius]

this PR should not allow props to be parented to holos that are parented to non CPPI entities

This is quite a bit more complicated to handle, as you need to recursively check the parent chain, check each link in the chain to see if it's a holo, and then find its first parented entity that isn't a holo to check CPPI

[Derpius]

QA Result: Passed with warning ⚠️ (see below)

Prop protection used: Forked NADMOD w/ admin bypass removed

• When using holoParent(n, e)
  • I can parent a hologram to my own entities ✅
  • I can parent a hologram to my own player entity ✅
  • I can parent a hologram to entities owned by a player who has given me CPPI permissions ✅
  • I can parent a hologram to a player who has given me CPPI permissions ⚠️
    • This is not possible with NADMOD at least as it doesn't let you affect player ents even when they buddy you
  • I cannot parent a hologram to entities owned by players who have not given me CPPI permissions ✅
  • I cannot parent a hologram to players who have not given me CPPI permissions ✅
  • When I fail to parent a hologram due to permissions
    • and I'm using @strict, an error is thrown ✅
    • and I'm not using @strict, no error is thrown but the holo is not parented ✅
• When using holoParentAttachment(n, e, s)
  • All above cases passing, except the warning on parenting to other players

For the issue with parenting to players who have you buddied, I see the following options:

• Add some custom logic separate from E2Lib.isOwner which handles this case manually (very bad!)
• Modify E2Lib.isOwner to support this use case
• Accept that some (probably most) prop protections do not allow tooling player entities even when you are buddied

[Hazeofdream]

This should minimise impact to non-malicious usage of this functionality

This isn't the job of wiremod

I can already name countless things this would break in my contraptions, Furthermore the literal warning of propcore is Allows players to manipulate props, including in other player's faces

[Denneisk]

This isn't the job of wiremod

Why wouldn't it be? This seems like an exploitable feature and an oversight.

I can already name countless things this would break in my contraptions

What are your contraptions doing that you need holos to be parented to random players?

[Derpius]

This isn't the job of wiremod

@Hazeofdream to add some more detail beyond what @Denneisk said, other cores (like propcore) use CPPI checks for its methods already, so the precedent is already in Wire to implement this. Additionally, the E2Lib.isOwner method would not exist if this was not meant for Wire.

Furthermore the literal warning of propcore is [...]

Propcore actually already performs CPPI checks the same way I'm performing them here (with E2Lib.isOwner), the issue is hololib.

[Hazeofdream]

What are your contraptions doing that you need holos to be parented to random players?

For many of my vehicles in particular, I use holos to bypass not being able to sound play on other people directly, many of these are audio warnings as a balancing factor.

Some of my other ones are visual aids like party systems, some are game recreations like battlefields 3d spotting mechanic

Some are workarounds, like using a parented holo with a turret to simulate damage, which iirc e2 still doesnt have

Overall this will cause people to get angry, this hasn't been a problem for years and server admins deal with the niche cases themselves, some servers revolve around this form of "abuse". "Solving" this will simply cause people to write yet another workaround that e2 is famous for, or use starfall to replace this issue

[deltamolfar]

Overall, as an option this is good PR, but imo it should be bounded to CVAR and disabled by default, and maybe throwing a warning into the server console on start.

[Fasteroid]

What are your contraptions doing that you need holos to be parented to random players?

For many of my vehicles in particular, I use holos to bypass not being able to sound play on other people directly, many of these are audio warnings as a balancing factor.

Some of my other ones are visual aids like party systems, some are game recreations like battlefields 3d spotting mechanic

Some are workarounds, like using a parented holo with a turret to simulate damage, which iirc e2 still doesnt have

Overall this will cause people to get angry, this hasn't been a problem for years and server admins deal with the niche cases themselves, some servers revolve around this form of "abuse". "Solving" this will simply cause people to write yet another workaround that e2 is famous for, or use starfall to replace this issue

I agree with pretty much all of these points. Preventing holograms from being parented to arbitrary entities would also break the grappling hook on my Wrecking Ball mech—or at least make it much worse should you try to attach to an unfrozen entity you don't own.

This PR won't solve what it's meant to either—I can just as easily holoPos and holoAng the hologram myself to do a "fake" parenting to an entity—it will just lag one tick behind. And then what do you do?

[Fasteroid]

Overall, as an option this is good PR, but imo it should be bounded to CVAR and disabled by default, and maybe throwing a warning into the server console on start.

This on the other hand isn't a bad idea

[MorlandMoran]

This whole pr is stupid and makes no sense, e2 has been like that for over a decade and now that e2 is barely used anymore you guys want to restrict even more instead of expending its use. To me it simply looks like another useless pr to fill your ego.

[shadowscion]

This isn't the job of wiremod

@Hazeofdream to add some more detail beyond what @Denneisk said, other cores (like propcore) use CPPI checks for its methods already, so the precedent is already in Wire to implement this. Additionally, the E2Lib.isOwner method would not exist if this was not meant for Wire.

Furthermore the literal warning of propcore is [...]

Propcore actually already performs CPPI checks the same way I'm performing them here (with E2Lib.isOwner), the issue is hololib.

This would simply break a lot of creative contraptions that I've seen over the years, anything from custom items to minigames to fully simulated combat systems. I think that should be expanded upon in your section on "Impact".

Ideally this would have been done 15 years ago, but it wasn't. If anything the longstanding precedent is that wiremod is not an admin mod, and if the goal is to stop blinders, this won't do it anyway... in fact I'd estimate that requiring prop protection now for obscure stuff like this actually opens up more avenues for abuse when you inevitably forget who is on your list.

[Derpius]

wiremod is not an admin mod

Why does propcore do the same check then

[shadowscion]

Genuinely "because that's the way it's always been". Forcefully changing it now just seems unnecessary. I liked the server convar idea, please consider giving people that option.

[Derpius]

Given the somewhat surprisingly controversial nature of this change, we've decided to move ahead with our own fork of Wiremod (for a number of other benefits beyond this too).

I'm closing this now given it wont be merged as-is and I'll be dropping the branch downstream. Having said that, everyone is obviously welcome to take the feedback here to implement a solution appropriate for the standard version of Wiremod.