TTT: Replace most UniqueID usage with SteamID #1182

Merged
merged 2 commits into from May 26, 2016

Projects

None yet

7 participants

@bmwalters
Contributor
bmwalters commented May 22, 2016 edited

Replaces lots of UniqueID usage with SteamID, preserving all necessary functionality.

@willox
Collaborator
willox commented May 22, 2016

I can see quite a few of these changes not being compatible with certain addons

@Kefta
Contributor
Kefta commented May 22, 2016

Fix 'em

@bmwalters
Contributor

I'll find out what changes break what and reply here.

@bmwalters
Contributor
bmwalters commented May 22, 2016 edited
  • DNA Sample field killer_uid -> killer_sid change breaks nothing (on GitHub)
  • SCORE.Events uid -> sid field change breaks nothing (on GitHub) (surprisingly)
  • ttt_transfer_credits concommand data change breaks nothing (on GitHub)
  • KARMA.RememberedPlayers index change breaks nothing (on GitHub)
  • GiveEquipmentWeapon timer name change breaks nothing (on GitHub)

but the big one:

Now so far this breakage are fairly minor as I doubt any servers use those addons (with the exception of the last one, which should no longer be used anyway). However there are some extremely major addons that this change breaks:

However, these addons only rely on this field in one or two places each. Simple changes can be made to fix them. But there is one more broken addon that will definitely cause grief:

  • TTT Defibrillator.

This addon is found on many servers and all of them must use this feature to operate successfully. The two versions that are on GitHub (@willox' and another one) are both broken by this, for example.
This will probably cause problems, as there are also versions on the workshop.

I believe this is enough for me to revert that one change. Any additional thoughts?

@Kefta
Contributor
Kefta commented May 22, 2016

Revert it, submit pull requests to all of the addons changing that one thing, warn people of deprecation. We can tell people how to fix it if they ask on Facepunch. I can help contact the authors on Steam if you wish for the workshop versions.

@robotboy655 robotboy655 added the TTT label May 22, 2016
@bmwalters
Contributor

Reverted.

@Bo98
Contributor
Bo98 commented May 22, 2016 edited

How about, for backwards compatibility, just simply defining both a field for UniqueID and Steam ID (with a note telling developers to use the Steam ID one) and using the Steam ID one throughout TTT?

rag.uqid = ply:UniqueID() -- Backwards compatibility: use the SteamID field below instead.
rag.sid = ply:SteamID()

That should get the best of both worlds.

@bmwalters
Contributor

@Bo98 Good idea. Done.

@svdm svdm and 1 other commented on an outdated diff May 25, 2016
garrysmod/gamemodes/terrortown/gamemode/corpse.lua
@@ -380,7 +380,9 @@ function CORPSE.Create(ply, attacker, dmginfo)
-- flag this ragdoll as being a player's
rag.player_ragdoll = true
- rag.uqid = ply:UniqueID()
+ rag.sid = ply:SteamID()
+
+ rag.uqid == ply:UniqueID() -- backwards compatibility; use rag.sid instead
@svdm
svdm May 25, 2016 Collaborator

You've got an = too many there

@bmwalters
bmwalters May 25, 2016 Contributor

How embarrassing. Fixed.

@svdm svdm merged commit eaf7d98 into garrynewman:master May 26, 2016
@bmwalters bmwalters deleted the unknown repository branch May 26, 2016
@markusmarkusz
Contributor

This killed every usage with bots and every equipment menu reskin and every custom score event. :(

@robotboy655
Collaborator

Should've used SteamID64 tbh

@markusmarkusz
Contributor
markusmarkusz commented Oct 22, 2016 edited

This would also break things. SteamID64 returns no value for bots on client state.

@markusmarkusz
Contributor

What can be used to identify players and bots on client and server?
Player:AccountID() doesn't work with bots.
Player:SteamID() has two problems: On server every bot has “BOT“ as SteamID. On client it returns no “NULL“ for bots.
Player:SteamID64() has one problem: On client it returns no value for bots.
Player:UniqueID() can cause problems.
I assume that Player:UserID() wouldn't be a good solusion.

@Kefta
Contributor
Kefta commented Oct 24, 2016

There isn't any default solution for unique IDs that handle bots included, unfortunately. It would be fine if the SteamID function had a separate numerical universe for bots or something that would solve conflicts, but I doubt it'd ever be changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment