Closed Kefta closed 11 months ago
I think it'd be preferable to just fix https://github.com/Facepunch/garrysmod-issues/issues/2771 instead of making them nullable though. I don't see any detriment to making them not a singleton.
The last time we tried to address https://github.com/Facepunch/garrysmod-issues/issues/2771 addons broke (with effectdata's specifically). So here we are.
I am not sure there's much of a point adding IsValid to these, as they can only be invalid when storing a reference to these objects from a hook argument and using it outside of the hook.
I'm not sure how addons would break from that change unless they're doing very weird things with the single instance, but if that's the case, then how about a bool argument to EffectData() and DamageInfo() to create a new instance? false = use the singleton (default), true = new object that is not invalidated across ticks.
The last time we tried to address Facepunch/garrysmod-issues#2771 addons broke (with effectdata's specifically). So here we are.
I am not sure there's much of a point adding IsValid to these, as they can only be invalid when storing a reference to these objects from a hook argument and using it outside of the hook.
Say you have a library function from an external addon that takes a UserCmd. Someone stores a cmd from a hook argument, waits until it's invalid, then passes it to the library func. That library func should be able to tell if the cmd is invalid, and if so, throw a proper error with a stack trace that doesn't put the library func at fault but rather the caller. Technically you can tostring the cmd and check if it has the invalid string, but that's not documented thus it's not guaranteed to work between updates.
Added IsValid methods to CMoveData/CUserCmd/CEffectData/CTakeDamageInfo
Any NULLable object should have an associated IsValid method.