[PATCH] Register all component classes as global

[sanderd17]

When someone makes a mod now, they have to copy the entire component over to their mod (which can be quite hard to maintain).

By registering all components as a global, mods can just extend functionality from the existing prototype in the public mod by doing that in a different file that's loaded afterwards (f.e. by creating an Attack.MyMod.js file to extend the Attack prototype defined in Attack.js).

There's a separate Engine function now to register variables as global (Engine.RegisterGlobal(name, var)), but it would be better to put this functionality inside the Engine.RegisterComponentType() method, so the actual JS components don't have to be modified.

As the schema gets saved to the Engine on RegisterComponentType, we should also allow to register the same component multiple times to allow schema overriding by mods (this might be less simple than the other part to get it right).

[Niek]

Didn't it work already?

[sanderd17]

Apparently not

[Niek]

Replying to sanderd17:

Apparently not

But I extended the Aristeia scripts like the way you described, and no errors are thrown...

[Radagast]

That's strange. It didn't work for me. It rather was random. Did you test the healing priests/converter unit after your changes?

(that's the only unit that uses the Convert Attack. Which for me didn't work.)

[Niek]

Replying to Radagast:

That's strange. It didn't work for me. It rather was random. Did you test the healing priests/converter unit after your changes?

(that's the only unit that uses the Convert Attack. Which for me didn't work.)

No, I did only test the cheat.

[sanderd17]

The cheat isn't a component.

[Radagast]

Attached a patch. Still I'm not really sure about this. Because my warnings were shown from random files mixed. As if they only overwrote temporarily. I will have to investigate.

And then if the GlobalRegistering should prove obsolete, then we might have to remove it again. This time with a regular expression. It's always at the end of the file. So this shouldn't be too difficult (if potential following blank lines also are matchin the regex.)

[sanderd17]

Euhm, I'd like to see the RegisterComponentType method modified, rather than changing every component file.

[Radagast]

oh ... yes .. I remember :D sorry.

[Radagast]

Note i might have mistake in because the linker forced me to change several parameters. Perhaps this was a random error. If so, then I should check. Will do.

[sanderd17]

Looks good, the TODO is unnecessary (I think this will be quite permanent). Now the other part ;)

[Radagast]

Will remove TODO then.

For part 2 I thought of splitting the C++ Script_RegisterComponentType() into 2. Is this fine?

We could also trigger hotloading but this would override all other mods' components with this IID and thus we needed a spearate flag to skip this hotloading branch (which is entered if the IID_Component already is registered).

Currently we return here:

if (!componentManager->m_CurrentlyHotloading) 
    return /*after error message: component already registered*/;

So either we split the C++ method in two. Or we introduce a flag that tell the function now not to delete/reset the component but only to add the schema.

Will have to create a patch.

[Stan]

I don't think Hotloading is a good idea, because that would bring more reason for the program to crash.

[Radagast]

The hotloading is reloading all files/components. We in our case need to just overwrite the schema of the first component that was registered (partially overloading of components if they have the same IID and name). But I still have not completely grasped the consequences. I will have to experiment with it I'm afraid.

You're right it's usually troublesome. 0AD has apparently solved this properly as there are few issues I can recall here.

[sanderd17]

I gave it a shot, added a new ReRegisterComponentType method (the RegisterComponentType has been renamed to RegisterNewComponentType, but the JS version of that name still needs to be updated).

RegisterNewComponentType makes it global, and has the same limitations as the old method (not able to overwrite f.e.).

The ReRegisterComponentType makes you able to overwrite existing stuff (and even fails if you don't overwrite something existing), but doesn't make anything global.

This is still very untested, but it seems to work in the most basic cases: overwriting methods, schema's and On... message catchers.

Hotloading needs to be tested more (what happens if you hotload the original file, the replacing file, what should we support?).

[sanderd17]

Not that simple after all

[Radagast]

ups. Didn't realise your effort. thx sander. Yet here is my patch. Just re-using the hotloading functionality instead of a separate ReRegisterComponentType.

Essentially just had to comment one if condition out.

Tested with Converting priests.

Nevermind yours sounds interesting too. A separate function was what I also thought of but I was never sure if I not missed something, so I just opted for the attached patch. But I'm for your patch as now you have had the work with it (and it's more probable to work properly. thx again for fixing. This is huge improvement for modding.).

[sanderd17]

In 15150:

Mark all components as global variables and allow to re-register existing components to update their schema and/or message receiving methods.
This allows for components to be extended in other files, so mods don't have to overwrite entire components.
Fixes #2517. Thanks to Rada for helping on this
(also fix some tests broken in the previous commit)