Page MenuHomeWildfire Games

simulation: Use Object.create(parent) to create inheriting objects
ClosedPublic

Authored by Krinkle on Aug 31 2019, 6:12 PM.

Details

Summary
var generic = {
  name: null
  abc: 42
};

var alice = Object.create(generic);

This is how prototypal object inheritence normally works.
It is similar to classical inheritance in other languages, but
works directly from one hashtable to another, and the link is
live (not a copy or merge).

Using new Foo(x) in JavaScript basically does the following:

  • obj = Object.create(Foo.prototype);
  • Foo.call(obj, x)
  • return obj

Despite prototypal inheritance being at the core of the language,
it was not exposed directly in the early versions of the standard
library functions. Instead, a funny trick emerged:

function my_create(parentObj) {
  function Dummy() {}
  Dummy.prototype = parentObject;
  return new Dummy();
}

var generic = {
  name: null
  abc: 42
};

var alice = my_create(generic);

In the early days, before this trick become well-known, people
really wanted to have class-like behaviours in JavaScript including
to be able to extend another class. Short of a built-in way to
do it, it became common to do:

ChildClass.prototype = new ParentClass();

This has the downside of needlessly creating an instance,
requiring the constructor to be executed and with valid arguments,
and in addition to creating an object that inherits ParentClass.prototype,
the object is also given the instance properties that the constructor
might create which can be really confusing at times.

Test Plan

Tests pass. Stuff works.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Krinkle created this revision.Aug 31 2019, 6:12 PM
Owners added a subscriber: Restricted Owners Package.Aug 31 2019, 6:12 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/15/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/524/display/redirect

elexis accepted this revision.Sep 2 2019, 7:18 PM
elexis added a subscriber: elexis.

IRC discussion at http://irclogs.wildfiregames.com/2019-08/2019-08-31-QuakeNet-%230ad-dev.log

15:56 < Krinkle> Taking 'PetraBot.prototype = new API3.BaseAI' as example. The reason this works is because an instance of a class will naturally be an object that has 2 characteristics 1) it's not a by-reference pointer to the parent class so you can safely add more prototype properties later and not break the parent class, and 2) it is an object that inherts all the properties from the parentt's class constructor.
15:57 < Krinkle> but it also means it executed the constructor function for no reason, and (confusingly) now means your subclass has properties visible on its prototype that were created by the parent constructor and don't belong on the prototype. Not to mention the mess of maintaining consructor args or avoiding them.

On the last part:

(confusingly) now means your subclass has properties visible on its prototype that were created by the parent constructor and don't belong on the prototype

Without knowing the involved code, there could also be the chance that it is intentional that the properties created by the API3 constructor are supposed to be on that prototype.

So it seems just testing that the AI doesnt crash or fail obviously seems insufficient.

But the constructor is calling the parent constructor, so the according properties inherited from the once constructor-called JS object will be 'overshadowed' (owned properties with the same name), so the result is the same.

I've done some archeology:

  • test file reference comes from rP7259 (2010)
  • "initial terrible AI player scripts" have it since rP8891
  • jubot has it since rP9378
  • qbot has it since rP10654
  • qbot-wc (later aegis) had it rP12267
  • aegis has it since rP13683
  • petrabot has it since rP14865

The first commit that used prototype = Object.create() was rP14582.
It might have been done to rewrite that isGo function simpler, differently, noticing that one can just link the prototype instead of calling something else. It seems to have been in the course of #2372. I didn't investigate further.

So I've tried anything I could conceive on the few hours spent in these two days to find any possible issue with this patch.

Thanks Krinkle for lending your JS wisdom and finding an antipattern that was copied around by every wfg AI dev without being noticed!

binaries/data/mods/_test.sim/simulation/components/test.js
21 ↗(On Diff #9551)

This can be tested by running the test binary, noticing that it fails hard when commenting out the line and passing when it exists.

binaries/data/mods/public/simulation/ai/petra/_petrabot.js
24 ↗(On Diff #9551)

Likewise by starting an AI match and seeing that it fails much, much more terribly when comenting out and succeeding with the applied patch.
Actually the AI just idles if the line is removed.

This revision is now accepted and ready to land.Sep 2 2019, 7:18 PM
This revision was automatically updated to reflect the committed changes.