bugfix(object): Fix crash when an object is created with veterancy and an upgrade that applies a terrain decal

[Mauller]

• Fixes: Game Crash in void Drawable::setTerrainDecal - Chemical suit + Instant build infantry - Mod Map #999
• Relates to USA Pilot has non-functional Chemical Suit upgrade and crashes the game GeneralsGamePatch#404

This PR fixes a game crash caused when a unit is created with veterancy and the object has an upgrade unlocked that attempts to apply a terrain decal. This can be reproduced with the chemical suits upgrade of the USA and is likely the reason why the chemical suit upgrade is disabled on USA pilots.

When an object is created the thing factory runs through all of it's behaviour modules and calls their onCreate() function if there is one available.

Units created with veterancy have the veterancyGainCreate module applied to them. This module iterates over all of the objects behaviour modules and forces them to update their status.

If the chemical suits upgrade or any other upgrade that applies a terrain decal is unlocked then it will crash the game. This is due to the objects drawable not being applied yet.

Normally the object will get its drawble created and bound to it after the behaviour modules have been iterated over. But to get around the veterancyGainCreate issue, we call the sendObjectCreated() at the end of the objects constructor.
originally it would be called within initObject that gets called after the behaviour modules onCreate loop within ThingFactory::newObject.

This then allows modules that would update the drawable to do so without crashing the game.

This is also fully retail compatible due to the ordering being maintained in the original initialisation sequence beyond creating the drawable and applying it to the object.

[Mauller]

Updated now.

[xezon]

How about we rename GameLogic::sendObjectCreated to GameLogic::sendObjectInitialized, which modifies the object and drawable states. Called from Object::initObject.

And then we create a new GameLogic::sendObjectCreated that allocates the drawable and binds object and drawable together. Called from Object::Object.

And GameLogic::sendObjectDestroyed stays as is. Called from Object::~Object.

That would be logically cleaner and fit more into the original code design.

Unless there is a reason why we cannot create a drawable inside constructor of Object. But that looks to be ok.

[Mauller]

if anything, the drawable could be set within GameLogic::friend_createObject it can't be done within the objects constructor since it won't have the template data at that point to set the relevant drawable information.

The object and drawable binding also can't happen too soon otherwise it changes the order of execution of some other operations and can cause it to mismatch with retail.

Which was why i just create and set the drawable early but don't fully bind or initialise it till it would happen within the original code.

[Mauller]

Just looking at this more, the thingFactory is managing the creation of the object and drawable. Using templated data to initialise them to the correct object / drawable.

I don't think it is necessarily going to be any cleaner to do it from within Object::Object since the thing factory still needs to get called and needs to be known about by an object then.

[xezon]

Please check if this change works as a fix:
xezon@f563b0e

I tested this against the Golden Replay and it passed.

It also looks as if it should prevent the crash and it would be the cleanest solution I can think of.

[Mauller]

Yeah this tweak appears to help resolve the issue without mismatching.

Originally i tried moving the call to obj->initObject(); within ThingFactory::newObject to be before the behaviour module onCreate() loop, but that caused mismatching. So considered that anything within it might casue a mismatch if performed too soon.

Will update the PR and push the changes with the extra context comments in a sec.

[Mauller]

quick rebase before making tweaks

[Mauller]

Tweaked with recommendations and tested against multiple replays.

I created a replay with the fix and created troops with chemical suits, then ran it back in a build without the fix and it did not mismatch.

[Mauller]

Tweaked based on feedback, should be good now

[Mauller]

Ditto

[xezon]

Is the Description of this Pull up-to-date?

[Mauller]

Is the Description of this Pull up-to-date?

Yes i already tweaked it.

[xezon]

Looks good.