wurstscript / WurstScript

Programming language and toolkit to create Warcraft III Maps
https://wurstlang.org
Apache License 2.0
225 stars 30 forks source link

Possibility to override parent class variables with child class variables in super constructor #952

Open TheSkullbot opened 4 years ago

TheSkullbot commented 4 years ago

Is your feature request related to a problem? Please describe. Unless I'm doing it wrong, you can't override parent (abstract) class member variable from the child class

Describe the solution you'd like Being able to override (abstract) parent member variables with child class member variables of the same name, maybe with an "override" keyword This would enable things like this :

public abstract class CustomUnitDefinition extends UnitDefinition
  string name = "Default Unit Name"
  construct( int unitId, int origID )
    super( unitId, origID )
    setName( name ) // Since you can't instanciate CustomUnitDefinition , setName should be called with the value of MyUnit.name

public class MyUnit extends CustomUnitDefinition 
  string name = "Child Unit Name" // Should override CustomUnitDefinition .name
  construct( int unitId, int origID )
    super( unitId, origID )
    // setName( "Child Unit Name" ) gets called from parent CustomUnitDefinition construct()
Frotty commented 4 years ago

Being able to override (abstract) parent member variables with child class member variables of the same name, maybe with an "override" keyword

Did you try using the override keyword?

https://wurstlang.org/manual.html#inheritance

TheSkullbot commented 4 years ago

override does work for methods/functions overrides, but not for members variables override VSCode displays this error :

2020-05-18 22_28_03-● Founder wurst - scripts - Visual Studio Code  Administrator

Frotty commented 4 years ago

Yea okay, that doesn't work and isn't intended to. In your example you seem to have a setter, which you could override.

TheSkullbot commented 4 years ago

I came with this solution for now, but this leads to duplicate code, as the overriden setter (here setDefault() ) would be the same in every child class That's the point of this feature request, being able to factorize such code :

2020-05-18 22_51_38-● Chicken wurst - scripts - Visual Studio Code  Administrator

The goal I would like to achieve is to set basic Unit info just by filling the matching variables (name, hotkey, etc) in the child class definition And I think a way to override parent variables from child class would solve this.

peq commented 4 years ago

If you want to change the value of a field, just assign it a new value. I don't see what this has to do with overriding.

If you need to change the variables before the super constructor is called, that would be related to feature request https://github.com/wurstscript/WurstScript/issues/158 .

TheSkullbot commented 4 years ago

I (mis ?)used the override word because I wanted the parent variables to be "overriden" by child variables of the same name, even in the super constructor.

In PHP (yeah I know), this works :

class A {
     protected $myVar = "Joe";
     public function printName() {
          echo $this->myVar;
     }
}

class B extends A {
     protected $myVar = 'Ana';
}

$test = new B();
$test->printName(); // prints 'Ana', but in Wurst/Jass it prints 'Joe'

In other words, I "just" want B.myVar to override A.myVar, if defined.

I think #158 won't solve the issue directly but may help, because it involves modifying parent variables before super constructor call.

Frotty commented 4 years ago

Overriding variables doesn't seem like something desirable. It's not obvious from your code sample whether/how the values are used, but it sounds more like a job for a builder pattern or getting rid of the variable in favor for a "getName" function that is overriden.

TheSkullbot commented 4 years ago

Yeah, it should not be the default behaviour, that's why I think an explicit modifier should be used.

In C#, this would be the new modifier It could also be nice to enable the access to the parent class original property/method, like through the use of the base keyword

The goal is to keep the child class as light as possible, by just filling the variables and let the parent constructor/methods use them, thus reducing code duplication.

Here's a "complete" example :

2020-05-19 09_45_17-● SkullUnitDefinition wurst - scripts - Visual Studio Code  Administrator

2020-05-19 09_47_51-● Pig wurst - scripts - Visual Studio Code  Administrator

peterzeller commented 4 years ago

The new keyword in C# seems to do something different: it is just for hiding variables in the super class, so you would have two variables with the same name.

The base keyword from C# is super in Wurst (see https://wurstlang.org/manual.html#super-calls).

I still don't understand why you cannot simply move the variable assignments to the constructor?

class PigDefinition extends SkullUnitDefinition

    construct(int unitid)
        super(unitId, UnitIds.pig)
        train = "Breed a"
        name = "Pig"
        ...

Maybe you don't even need a subclass and can just do:

init
    let pig = new SkullUnitDefinition(...)
    pig.train = "Breed a"
    pig.name = "Pig"
    ...
Frotty commented 4 years ago

I still don't understand why you cannot simply move the variable assignments to the constructor?

Doesn't work if those values are used in the super-constructor.

TheSkullbot commented 4 years ago

I still don't understand why you cannot simply move the variable assignments to the constructor?

That's the best possible solution with the current language features, yes. Not as sexy as I would like it to be, but it works. But that's why this issue is a feature request and not a bug report :p

2020-05-19 10_50_39-● Pig wurst - scripts - Visual Studio Code  Administrator

Frotty commented 4 years ago

That's the best possible solution with the current language features, yes. Not as sexy as I would like it to be, but it works.

I suppose what's best and the sexiness here lies in the eye of the beholder. I personally would prefer a different solution here. If you don't need access to the PigDefinition class/type at runtime, you could also get rid of it completely in favor of a builder pattern (see cascade operator).

But that's why this issue is a feature request and not a bug report :p

Fair enough.