PlayerRemoving -> PlayerRemoved

Feature Requests Engine Features
#1

Simple suggestion: PlayerRemoving should be deprecated in favor of PlayerRemoved to be consistent with ChildRemoved.

6 Likes
Implement PlayerRemoved
#2

I think “Removing” doesn’t make much sense because events are past-tense rather than current-tense verbs. However, there are multiple instances of this it seems:

  • CharacterRemoving
  • DescendantRemoving
  • ServiceRemoving (I didn’t know this was a thing)
  • PlayerRemovingLate (corescript only)

There are many more instances of “Removed” though, so I guess these should all (or at least the first two) just be updated to reflect the standard naming convention.

4 Likes
#3

I believe it’s called removing because it happens while the player is leaving. For example if the player had already left, then providing the Player object as an argument would be useless, since it would have already been destroyed.

16 Likes
#4

I was about to post about this - yes, you are right. These should remain named the same.

2 Likes
#5

After testing this I here’s what I’ve gotten:

Exhibit A; player kicked via game.Players.LocalPlayer:Kick(), client side.

Exhibit B; player kicked via game.Players.Player2:Kick(), server side.

Exhibit C; player was kicked via game.Players.LocalPlayer/Player1:Destroy() (either method works).

So it seems it takes precedence over things such as Destroy and possibly any other method of removing the player. Just wanted to add in this extra information.

EDIT: Here’s the code I used.
image

1 Like
#6

You can read and write to objects being removed (it’s not limited to just Players) because references to objects in Lua are not GarbageCollected. If you keep a table of objects, the objects will never disappear from the table and you can read/write them ad infinitum.

#7

OT: would not recommend doing that

#8

I’m aware of this. However I was trying to point at how the implementation of it means that even if players are removed in any sort of way you’ll have about a frame to access everything you need from them (since things like Backpack will be lost if not referenced right away).

#9

While it works it saves me time and eyesores while writing, even if minimal.

Also they did talk about it here and even if it may be a side effect I can’t see a way in which it’d break (since __call and __index both work practically the same way as metamethods, and this is based of both of them).

#10

__namecall isn’t based on __call or __index.

Specifically, the way __namecall works right now is:

  • We do not morph Lua bytecode nor do we introduce new opcodes; you still have OP_SELF+OP_CALL for a colon-call, with argument evaluation inbetween.
  • OP_SELF checks if object has __namecall impl and if it does, sets up the stack to have two arguments - object and method name - instead of the usual result of the table dereference and an object
  • OP_CALL recognizes this stack setup and reshuffles the arguments to perform __namecall

Here’s an alternative implementation that we initially planned to do:

  • Reorder opcodes so that arguments are computed first, followed by OP_SELF, followed by OP_CALL (requires changing Lua compiler slightly)
  • Change OP_SELF to check if the next insn is OP_CALL and the object implements __namecall; if so, call it directly

This changes evaluation order for cases where computing the object requires calling a function but it doesn’t seem a significant breaking change.

Here’s another alternative implementation that I was originally thinking of:

  • Reorder opcodes so that arguments are computed first
  • Introduce a OP_NAMECALL opcode, that combines OP_SELF + OP_CALL above (requires changing Lua bytecode format)

There probably are other ways to implement this. These have subtle semantic and performance differences (we lose a tiny bit of performance right now because we decided to not change Lua bytecode, but we preserve semantics of otherwise valid code) - this can change at any time because we decide that we want to optimize things better or want a different tradeoff or something. Do NOT rely on this.

4 Likes
#11

Ah, right, here’s one more option; I think we discussed this briefly after accidentally discovering during the codereview that game('GetService', 'Players') now works and decided to not do anything about it for now:

  • Keep existing __namecall implementation, but introduce OP_NAMECALL instruction that matches current OP_CALL semantics
  • Don’t have any “magical” behavior in OP_CALL; compiler will ensure that OP_SELF is always used with OP_NAMECALL
  • OP_NAMECALL will fall back to usuall call if OP_SELF didn’t set up the stack properly

This would have preserved the semantics perfectly (AFAICT) - game(...) would never trigger any code that knows about __namecall - but the attraction of the solution we ended up shipping is that it’s a runtime-only change that we don’t have to stage - we can enable or disable it any any point without changing the compiler or bytecode format.

2 Likes
#12

Alright, thanks for clarifying. I do agree that not changing the bytecode format is for the best (staying true to Lua, it’s like a meme here nowadays) but unless there is a drastic change I don’t see this really breaking. What I was trying to get at is how metamethods usually work by passing (self, arg1, args2, etc...) and how this:method() used to call __index prior to any change.

So the assumption I was making is that __namecall would work as any of the other metamethods so whichever way it’s triggered wouldn’t matter (whether it’s from __call or __index styles).

I would also like to add that changing or adding opcodes can be a little messy in the way that Lua is handled under the hood with some potentially sneaky bugs.