TeamUlysses / ulib

ULib: A Lua library for more rapid development on Garry's Mod servers
http://ulyssesmod.net
Other
104 stars 55 forks source link

ULib.replicatedWritableCvar no longer works with values as default_value #81

Closed ZenBre4ker closed 2 years ago

ZenBre4ker commented 2 years ago

Steps to reproduce

  1. Use ULib.replicatedWritableCvar("some_name", "other_name", true, true, true, "mysettings")

Expected behavior

The Cvar should just be registered as always no matter the value I give as default.

Actual behavior

The system crashes and the gamemode doesnt work anymore.

Error(s) in server console, if any

[ulib-master] addons/ulib-master/lua/ulib/server/util.lua:131: bad argument #1 to 'WriteString' (string expected, got boolean)
  1. WriteString - [C]:-1
   2. replicatedWritableCvar - addons/ulib-master/lua/ulib/server/util.lua:131
    3. AutoReplicateConVar - lua/ulx/xgui/server/sv_terrortown.lua:21
     4. init - lua/ulx/xgui/server/sv_terrortown.lua:145
      5. fn - addons/ulx-master/lua/ulx/modules/xgui_server.lua:314
       6. unknown - addons/ulib-master/lua/ulib/shared/hook.lua:109

Error(s) in player's console, if any

Version

ULib v2.70w ULX v3.73w

bigdogmat commented 2 years ago

The documentation says it should be given a string, https://github.com/TeamUlysses/ulib/blob/09c4e751a1deaec6202c7c5c23a8b11205720956/lua/ulib/server/util.lua#L98

The last change was for converting umsg to the net lib, my guess would be the umsg.String didn't error when given a non string. So while it may have worked in the past it was only because of chance.

Nayruden commented 2 years ago

As @bigdogmat points out, this function is documented as requiring a string.

ZenBre4ker commented 2 years ago

But guys, gmod is now 15 years old I am only asking to make a failsafe for all those old addons, that still use that? Just converting it with tostring() shouldnt be a problem, right?

TimGoll commented 2 years ago

I have to agree with @ZenBre4ker here. At least in the TTT community all addons are broken because of this change. However none is going to update these addons.

We get weekly bugreports about TTT2 being broken since you guys changed this. Even though TTT2 doesn't use ulx anymore.

JoshPiper commented 2 years ago

Considering TeamUlysses/ulx#195, and how ULX doesn't internally cast to strings either, perhaps documentation isn't enough, and this should be revisited?