So, I realise this isn't technically a bug on a code level, but it can certainly be seen as one by a modder: in ZScript, uninitialised variables default to a value from memory, rather than using a sensible default, which seems like weird behaviour for someone not familiar with the inner workings of memory (like, I'd reckon, the majority of people using ZScript). I think it'd be worth using a default value for uninitialised variables to make it easier for the majority of people using ZScript. Also, as dpJudas pointed out on Discord, "right now the "random" value seems to be related to register/stack allocation - change the VM backend and you'd risk changing it to something else".
As you have said, this is not a bug, and I vote for keeping things working as is if opposed to automatic zeroing or something. Don't get me wrong, uninitialized variables can be a pain, but it's minimal, and in result people can learn the details on why things are as they are. If you know you want a specific value, you should write it explicitly. The sensible default for integers vary depending on your environment - it could be zero, one, two, NaN, or all of them until deemed otherwise, the last of which is obviously not something that should be implemented in ZScript, but nevertheless a possible interpretation of a statement, semi-common from people who know their maths but not their programming.
If you want people to not get surprised, give them a warning while loading the mod - and if they like ignoring the warnings, they SHOULD learn more about programming. The big problem is that my current interactions with ZScript compiler suggest it is not capable of non-trivial code analysis and a simple if-delayed initialization would probably also give out such a warning, so some sort of notation to avoid it should be provided, at which point you could enforce it in the next version of ZScript, I guess. Of course some kinds of variables can't have an instant initialization yet which is actually a bigger problem which IMHO should be fixed first.
The non-determinism complaint is an interesting one, but I don't see a real problem being solved here. A modder can do stupid stuff anyway (for instance, in ACS, {GiveInventory("Clip", GetScreenHeight()));}). The real issue here is that they could do so without expecting it to become a problem, which would be solved with a warning, again provided they take it seriously which they should.
Frankly however, It's more of a feature request than a bug report, no?
@stan423321
This is inconsistent with other things in the engine which automatically initialize to zero, and it can cause bugs that are VERY hard to debug.
I'd say int variables should be initialized to zero and floating-point variables should be initialized to NaN.
The former doesn't make it super clear there's a bug if you forget to initialize it, but it'll at least be reproducible. The latter, meanwhile, is way more helpful, since NaNs propagate, meaning that whatever non-comparison math operations you do with the uninitialized variable will (well, they should) return NaN, meaning it should be pretty clear if you don't initialize a floating-point variable.
I wonder what the performance difference would be if things were automatically initialized all the time.
I agree this is more of a feature request, though one of the major benefits of implementing this would be for the bugs forum.
EDIT: I am now trying to figure out a seemingly randomly-occurring desync in Hideous Destructor. I have no idea if it's something in HD or a memory leak due to a GZDoom compile error or uninitialized variables being read differently on the 2 instances. (EDIT: Ironically this one was actually really straightforward...)
Given what is needed to keep multiplayer games in GZD in sync, I think having ZScript always initialize variables would be a huge boon to everyone.
I've added a warning every usage of uninitialized variable in 8104ef5.
Of course it cannot detect all cases of potentially uninitialized variables.
For example, passing a variable to a function as out argument counts as initialization. Compiler will remain silent even if no value is assigned to it.
I agree with Phantom, I think they should all at least automatically be zero'd out. It's just so much safer, especially for things like custom properties.
Ask yourself why C++ doesn't zero'd variable out by default. Modder doesn't bother to check warnings in console during production of his/her masterpiece, right? So let's move the cost of that laziness to end users.
C++ doesn't zero variables because it's a low-level language. ZScript, meanwhile, is a high-level language, and runs inside a VM.
Not zero-ing out variables is also extremely inconsistent with ACS and other parts of the engine, as they do zero variables out.
C++ isn’t a low level language as it allows very high level of abstraction. But it doesn’t initialize variables for the only one reason: performance.
Let’s say I’m fine with the overhead of variables’ initialization if you can convince Graf on this topic.
Alternatively, if there's a null or nil in ZScript, variables should probably initialize to that instead. A lot of languages do this, and it doubles as both an consistent, logical behavior and an debuggable error - after all, you won't get far in more languages by adding null and a number, or trying to get the fifth element of null.
This most definitely needs to be revisited now. Massive issues with uninitialized variables have been found, such as this, which should most definitely NEVER happen.
EDIT: Oh yeah, and now with asmjit, initializing variables is a single mov instruction. Which means it's literally in the order of nanoseconds.
Initializing to sensible defaults would be nice. Go does this; it works well. Interestingly Go has class-like things, but no constructors, like zscript. Having default values helps here... imagine you want a class that does lazy initialization, and don't want to give it a static "Create" fake-constructor. Well, it can have a bool "initted" field, which you'd know would start off false, so in your doStuff method you can just check the value of "initted" and init if needed. Now the consumer of your class doesn't need to know they need to use your static Create, they can just use `new`.
WRT choosing sensible defaults for each type, Go managed to figure this out (false, zero, empty string)...
Of course zeroing as "sensible defaults" only works as long as the defaults are actually sensible in context. The main point of having a constructor is to set something sensible in the current context.
The big problem here is of course that ZScript doesn't have real constructors yet. I never got that far and it's probably not something that's trivially added.
For actors this is taken care of by the 'Defaults' block, but not for other classes.
Graf Zahl wrote:Of course zeroing as "sensible defaults" only works as long as the defaults are actually sensible in context.
I'm not sure it needs to make sense in context to "work." Or rather, the context that it works in is the broader context of default values being "nothing" values, not the more narrow context of whatever the class is doing.
So instead of thinking to yourself "x is uninitialized," you think "x is nothing." What nothing actually is depends on x's type.
Constructors would be nice, but personally I'd rather see variables initialized to default values, since it can cover at least part of the initialization stuff that constructors would, and would reduce UB.