[BUG] Vars can be redeclared (UnrealScript)

[Feralidragon]

This compiles successfully, but it shouldn't:

class MyActor extends Actor;

var int Target;

function doStuff()
{
    Target = 123;
}

If I comment out the var int Target declaration, then the compilation will correctly fail stating that the type being assigned in Target = 123 mismatches its original type (which is Actor), and if I do that and set Target = self; it then compiles successfully as well, proving that Target was already declared before.

This is reproducible with any type of declaration for any var, and the same problem exists at least in 436, 469b and 469c (didn't test with 469a).

[SeriousBuggie]

This is not redeclaration. It is shadowing. And i think it is by design. But try find mention of this in EPIC docs and fail. In any case this behavior is standard now, de-facto at least.

If we change it, some code fail compile in v469c, but fine compile in v436.

IIRC variables packages determined on compile time, not on runtime. So any subclasses which use Target from Actor still use it, until not recompile it, if you introduce this int Target.

[Feralidragon]

OK, so it's a feature, not a bug.

Makes it easy to redeclare an existing var by accident though, which sucks.

You mentioned that some code will fail to compile if this is fixed: do you have any examples of that? I am genuinely curious to see what kind of real situation this type of code is justified.

[SeriousBuggie]

At least I implement Counter with variable for count as int, not as byte. For fix limit. But i am too lazy for rename var in code, so just simple copy all code and make variable int.

https://ut99.org/viewtopic.php?f=5&t=14773

class MyCounter expands Counter;

//-----------------------------------------------------------------------------
// Counter variables.

var() int       NumToCount;                // Number to count down from.
var   int       OriginalNum;               // Number to count at startup time.

[SeriousBarbie]

Makes it easy to redeclare an existing var by accident though, which sucks.

I found several cases where a programmer declares variables in child classes using the name of an already declared variable in a parent class but didn't realize that code of parent class uses the parent variable and not the one he has declared. See TeamMonster.u for example.

[Feralidragon]

Then, thus far, what's being implied here is that only code out there in the wild, and bad code at that, from mods at most, have stuff like this, right? At least I don't remember the base game having this anywhere (unless anyone has such an example).

If that's the case, wouldn't it be better to fix this, and fix those mods afterwards instead?

Because, as of now, as a personal experience (as someone returning to scripting after many years), for a lot of properties I declare in my classes, I have to double check the parent classes to ensure I don't accidentally redeclare a property.

If a full fix is undesirable because of old code, then maybe at least a setting in the compiler could be added to allow that problem to happen (similar to another setting that was also added with the latest patches), meaning that the compiler would then throw an error in this case by default, and if you wanted to compile that stuff in the newer compiler you could pass an explicit argument for the compiler to allow it.

[SeriousBuggie]

Nobody will mess with compiler option. And what if code compiled inside UnrealEd? How pass argument here? We definitely can not do "fix this + fix mods", just because this impossible. Nobody know where and which this appear.

About compiler warning or note - I think this technically possible.

[Feralidragon]

Nobody will mess with compiler option.

But the thing is, there's already a precedent for that with the compiler argument that allows you to bypass the validation of vars with the const modifier, in order to be able to compile code that writes directly to those vars.

You don't get that option from UnrealEd either, yet you need it to compile some mods too (although there's an alternative to that argument, but it's not pretty).

If in this day and age you're still compiling code like that inside UnrealEd, then maybe you should stop doing code like this? Laziness is not a good excuse to keep something that is obviously broken.

Although I do understand the need to be able to compile from inside UnrealEd at times, since I still do it myself whenever I need to MyLevel stuff into a map, or to do quick testing (which is how I found this issue to begin with), but if the idea is to support old external mods that depend on this in order to be compiled, those should not need to be compiled in UnrealEd at all, so that's not really a valid concern in this case.

Unless you mean mods that you don't have access to the original source code, which can be easier to modify from UnrealEd. But in that case, if possible, maybe some sort of field in the script view inside UnrealEd could be added so you could add compilation options, which would allow you to do that, and would even allow the case I mentioned above.

But, by default, leaving it like it is, is a really serious issue. It only makes coding harder in UnrealScript for no good reason at all, it's just a source of bugs which may be some of the hardest to figure out, especially by more inexperienced coders.

[SeriousBuggie]

But the thing is, there's already a precedent for that with the compiler argument that allows you to bypass the validation of vars with the const modifier, in order to be able to compile code that writes directly to those vars.

Yep. But it hidden option for internal usage atm. So better not write such info on pub repo.

If in this day and age you're still compiling code like that inside UnrealEd, then maybe you should stop doing code like this? Laziness is not a good excuse to keep something that is obviously broken.

Who decide what code broken and what no? i not see anything "broken" in this approach. Shadowing use in another langs and declared as feature, not as bug. So why I need thought about it as of a bug? Need strictly declared rules for that. Which nobody read. Even if you implement compiler warning, some ppl able do such stuff in v436 without any knowledge about how "bad" this is.

Although I do understand the need to be able to compile from inside UnrealEd at times, since I still do

If you not use, this not mean nobody use. For me easier do all stuff in UnrealEd. Like IDE concept. Because mess with ucc folder and external editor very boring. Maybe you setup fancy scripts or 3rd party IDE, but this not excuse for such message for all. All players mostly has UnrealEd only.

But, by default, leaving it like it is, is a really serious issue.

There thousand of this "serious issues" which need remember on mapping. bStatic on network. DT_Sprite and rotation replication, and so on. One of it this. Good if all be fixed. But not see how this issue more serious from other. You can with any screwed map easily.

[Feralidragon]

I got curious, and checked if UE2 has the same issue, and it has (not much changed from UE1 to UE2 on this regard), and went then to UE3 (which was a greater leap), but to test anything there is far more bothersome, so I ended not testing it (if anyone can confirm it in UE3, it would be greatly appreciated to understand the extension of this).

@SeriousBuggie The problem here can be broken down into the following:

• This is not "declared" as an UnrealScript feature anywhere, unlike other languages;
• You failed to provide an example of this being used in the standard game original code;
• The sole example you gave from custom code is a terrible coding example;
• Barbie mentioned that it seems this happens quite a lot in mods, but it seems to be always by accident;
• Yes, there are languages that allow this, but they also allow to disambiguate which one you are using (the only legit way I found of using them), UnrealScript doesn't.

Unless you have a legit example to show the usefulness of this type of feature, then for me it just does more harm than good, and Barbie has the mods to prove it.

Having said that, however, I will concede the following:

• Java seems to allow shadowing, and UnrealScript was heavily inspired by it;
• This problem has been around for over 20 years, and it's also present at least in UE2, so an argument can be made that it's not worth fixing it;
• The fact that a new property is actually created rather than writing to the old one makes it at least "safe" to an extent.

So I will leave up to the project leads what they want to do, although it's indeed very likely this issue will get closed as "won't fix" or "not a bug".

Although I still think that calling it a "feature" is a really big stretch, especially when such a feature is completely undocumented, apparently unused in the base game and incomplete.

[SeriousBuggie]

I never say it is good or must be used. Or used in base code. You ask for example - I show it. I not see any reason why such code can not be written in some mod. I just point to fact - you can not know used this or not in 3rd party code. For example, for me, destroy flag look like non-sense, but as Anth point later, exists Burger mod, based on this behavior.

There can be same. Anyway last word for Anth.

[SeriousBarbie]

Maybe I didn't get the problem, but for me it is apparent that a variable that is declared in a sub class has nothing to do with a variable with the same name in any parent class.

[Feralidragon]

Actually, the more I think about this now, and the more experiments I make, the more I have to concede to @SeriousBuggie that maybe yeah, this was likely an intended feature at first, given the likeness to Java this language was striving for at the time, and I do understand the benefits of being able to do it, especially if we consider cases of classes which have a set of simpler named properties in a separate category for UnrealEd (which does work).

So yeah, sorry @SeriousBuggie , I am starting to think that you're right on that one, as far as stating it as a "feature" goes.

But it's not completely apparent to the engine itself, because even as a "feature" it's fairly incomplete, because depending on the context you cannot really disambiguate, either directly, or even at all.

For example, let's say you do create a new class, and you declare another RemoteRole in your class, (extending from Actor), and then you need to set default values for the Actor's original RemoteRole and your class RemoteRole as well, how you would go about this? Is there even any possible syntax for it?

My own attempts all resulted in warnings from the compiler.

When opening the default properties in UnrealEd, both properties do show up, so you would think you would at least be able to set their defaults there... nope, it doesn't work.

You change them, you save the package, and when you reload the editor and package again, the default was still the same one as before, and not the one you changed it to.

Even if you export to an external uc file from UnrealEd, with the default value modified, the exported file only sets the default for the new property you created.

So it seems there's no way to change the defaults of the parent's version of the property of the same name. The same goes for config files.

Also, as far as disambiguation goes, it seems that you have no direct way of doing it: you need to use local vars to do the disambiguation, by creating a local Actor var, assign your class instance to it, and only then you can get the Actor version of RemoteRole instead of the one declared in your class.

Doing it like Actor(self).RemoteRole, for example, causes a compilation error.

As far as replication goes, compilation-wise it seems to work correctly, although I am not sure if it works well in actual replication, but if you have a new Role property in your class, you cannot really define replication rules with the Role condition (which is the most common type of condition), because you have no way to disambiguate correctly inside the replication block either.

So, now I do concede that it may have been worked as a feature, because it does work consistently in a lot of different scenarios (compilation-wise), but then it's a very broken feature and should not be used that being the case.

Alternatively, these issues could be solved instead, to make this actually a fully working feature, and the resulting packages would likely remain compatible with 436 as well, but as it is.... it seems just a source of potential bugs and headaches,

[SeriousBarbie]

A kind of syntax to access parent variables would be nice. Something like:

Class WhatEver expands StockClass;
Var Pawn Target;

function Test() {
    log("My Target=" $ Target $ ", Parent.Target=" $ Super.Target $ ", Actor.Target=" $ Super(Actor).Target);
}

[an-eternity]

Maybe there is something i do not understand about this issue... v469 (a, b, c) compiler now always warns if a re-declared variable exists, e.g.: "Warning, 'Target' obscures 'Target' defined in base class 'Actor'."

So with v469 it is unlikely possible to be unaware about any accidentally re-declared variable in your project. These warnings helped to fix a lot of old mods where modders just used to blindly copy-paste an entire code from the parent classes, including variables, which then was causing a specific problems in some cases (including the ones with default values, as said above)...

Actually, i also still can not imagine any case/example where such a feature would be necessary. And in all the cases i have seen these re-declared variables appeared by an accident, usually as a result of a blind copy-paste by a beginner modders. And in case if you need to re-declare a variable because another Type is needed, then it is better to use a new variable name instead, since using another Type for a re-declared variable may potentially bring a lot of hard problems afterwards...

[Feralidragon]

A kind of syntax to access parent variables would be nice. Something like: [...]

Yeah, that's the first syntax that came to my mind and tried, since it only seemed rational to follow the same logic as calling parent functions.

If that syntax was eventually supported however, while it would solve both the direct disambiguation and replication issues, it still wouldn't solve the defaults and config issues, they would need brand new syntax given that Super doesn't make much sense, especially for configs, and configs and defaults should share the same syntax, or at least be as similar as possible.

v469 (a, b, c) compiler now always warns if a re-declared variable exists

I didn't notice that, and yeah, it does. I am using UMake to compile from source code, and it hides warnings by default, but upon set it to display all compilation details, that warning does happen (didn't know that).

However, in UnrealEd you don't really have that warning at all.

i also still can not imagine any case/example where such a feature would be necessary

There is actually, at least in my opinion.

No one gave a legit good example thus far, but I did think of one that would come in handy to what I am currently developing: in my case, I am now developing a new FX package, and one of the classes is CoronaController (it actually has a C21FX_ prefix, but shortening it for this case).

This controller will be meant to control the properties of a new dynamic corona system I am developing (to replace the old one I made before), and it has some properties that go like this:

• CoronaTag ;
• CoronaTexture ;
• CoronaColor ;
• CoronaSize ;
• CoronaGlow ;
• etc (I guess you get the idea).

And all of these properties are editable from UnrealEd (since it's a mapping actor) and are under the "Corona" category. So when you open the "Corona" category of properties of this actor, these are the properties you see.

Now, imagine the same set of properties, but without the "Corona" prefix on them, but still be under the "Corona" category, so it would be, respectively:

• Tag ;
• Texture ;
• Color ;
• Size ;
• Glow ;
• etc...

From a mapper standpoint, which would be more readable, understandable, and overall pleasant to see and work with?

In my own opinion it would be the second one, so while for mods this doesn't have great use cases, for UnrealEd at least this is useful to create a set of properties, which are more simply named, under its own category, which from the mapper point of view is clearly a different section of properties ("Texture" from Display is different from "Texture" from Corona).

The problem, however, is what I mentioned before: the feature is incomplete and cannot be really used because it lacks the ability to be able to access the super class version of these properties in very important contexts (replication, defaults and configs), making pretty much the usage of this something more akin to an accident as you mentioned, and not a conscious decision to use the same name.

[Feralidragon]

Closing since this is likely just an incomplete feature, so a different (future) issue requesting its completion may make more sense down the road.

[SeriousBuggie]

I finally found live example inside UT. Bad one anyway, but...

542

This happens because class TrophyGame extends UTIntro; define own var LadderInventory LadderObj; and not bother init it at all. So this shadowed var LadderInventory LadderObj; from class UTIntro extends TournamentGameInfo; and never be loaded, so always be None.