Fix #175: Add Ghost Collidable check (& some properties)

[unknownersamli]

didnt change too much, else I'm not sure something will go bad

I think if the Ghost can interacting with you, then it can change how the map working

[RedFlames]

To be clear, only the changes that set Collidable are actual changes, right? The rest is just refactoring?

My first question would be, if someone has interactions off, so Ghosts are spawned non-collidable, then they turn on interactions but have stayed on a screen with other players -- would they now still be unable to grab the players on screen until those respawn?

I imagine setting Collidable should maybe then be done every time Update() runs,
but then you'd still need to keep in mind the "cooldowns" that turn it off upon throwing someone, and such.

Actually, I see now that it does indeed get set in Update() in line 196. Hmm.

Should it be

Collidable = Holdable.Holder == null && holdable; 

instead of

Collidable = Holdable.Holder == null && MutualInteractable; 

to honor the cooldown? I imagine it should be the inverse of the first check in OnCarry that returns early, after all.

Though I am also inclined to say I don't like how the boolean there with the local variable is three levels of "nested" conditionals, by including Grabable which includes MutualInteractable...

If this would solve all the issues that arise from Ghosts being Actors though, that'd be cool.

[RedFlames]

Okay, so reading through this again, and looking around a bit how the Holdable gets handled and how Collidable gets set around that... here's my thoughts now.

---

Before these changes, Ghost.Collidable basically followed this logic:

• set to true at Ghost creation
• set to false while Ghost is being carried by player (Holdable.OnCarry)
• set back to true upon Holdable.OnRelease
• ( plus there's that bit of extra logic in Update for the "forced release" )

So essentially, Ghost were always Collidable unless being carried by player.

And your logic of setting this on every Update tick:

Collidable = Holdable.Holder == null && ...

essentially just tries to stay in line with what OnCarry and OnRelease are already doing, right? Which as a whole it seems a bit unnecessary to me to be setting Collidable on every Update...

---

Meanwhile Ghost.Holdable follows this logic: (not directly related but relevant nonetheless)

• created during Ghost creation but not "Added"
• whenever Update sees that ghost is "holdable" and HoldableAdded is false, it adds it, sets HoldableAdded
• whenever Update sees that ghost isn't "holdable" and HoldableAdded is true, it removes it, unsets HoldableAdded

==> HoldableAdded boolean keeps track of whether or not it has been Added/Removed as a component of Ghost

---

My thinking now is this:

• don't update Collidable every time Update runs
• change OnRelease to set Collidable = Grabable rather than true
• during the parts of Update where HoldableAdded gets set/unset, also set/unset Collidable = holdable/false respectively

At least by my understanding this makes sense, we're trying to cover these cases:

• no Holdable -> no way Ghost would be carried(?) -> non-Collidable, the new logic we want
• has Holdable -> not currently being carried -> Collidable based on being holdable/grabable, rather than always true
• has Holdable -> is currently being carried -> non-Collidable as per OnCarry logic, stays unchanged

---

And additionally I also would like it if all the checks about if Holdable.Holder == / != null get replaced with Holdable.IsHeld since that's the same thing, but a bit clearer?

• Holdable.Holder == null ==> !Holdable.IsHeld
• Holdable.Holder != null ==> Holdable.IsHeld
  That's an existing issue with the code though, that's not something you introduced. Somehow there was only one place that makes use of IsHeld, pretty inconsistent usage

---

My only remaining question is, does Ghost need to be Collidable at all? Or because of the Holdable can it still be picked up even if the Ghost actor entity itself isn't collidable... I'd have to check but I'm guessing it could, maybe.
But that's just a concern of whether or not we want to change how CNet Ghosts interact with the game, huh.

Edit / PS:
I looked at this again because someone brought it up on the discord. At first I thought that person was the author of this, but it wasn't. :3