Pryaxis / TShock

☕️⚡️TShock provides Terraria servers with server-side characters, anti-cheat, and community management tools.
GNU General Public License v3.0
2.43k stars 382 forks source link

Naming conventions revisited: TSPlayer & TPlayer #1542

Open hakusaro opened 6 years ago

hakusaro commented 6 years ago

I'm not sure that we ever discussed this when implementing it. Are TSPlayer and TPlayer really the best names for TSPlayer and Player objects, respectively?

I understand that there's some level of verbosity we avoid in spelling out what objects are. However, I think it's probably really counterintuitive to new developers who look at this codebase what's going on. You can get really great references too:

Finally, though this isn't part of a discussion, we should really clarify how to canonically refer to players we're passing around. Right now, we have three really brilliant ways of doing nearly the same thing:

In my opinion, I think we should:

This all comes down to usability. We need to expose hooks consistently and pass data around consistently. A lot of the newer code already passes around the right object (TSPlayer) but a lot of legacy code swaps around player indexes like they're going out of style.

bartico6 commented 6 years ago

It would be a lot simpler if the naming within the game's code was akin to Minecraft's (or CraftBukkit's? I don't remember) - CraftPlayer can then be wrapped by the generic Player, and the entire conflict would be gone.

It would be nice if OTAPI/TSAPI could enforce this (name terraria player objects TPlayer or TerraPlayer) so that we can have the generic tshock player object named "Player" without conflicts.

If we cannot get this benefit, I think that renaming TSPlayer to something without a T (WrappedPlayer perhaps? idk) and then making the 'native player' named TPlayer/Player could work?

Or we can take example from CBukkit and just make it a .Handle or .Native getter to get the "native" "handle" to the player.

I can't decide which would work better. I'd personally opt with version 1 if @DeathCradle can arrange that, then if that's impossible - version 3 [more familiar to devs from other backgrounds] and then as a last resort version2. This will be a breaking change for all plugins though. [let's break it this too right now, if the next release breaks a lot anyway]