I think this is a recent regression, but I'm not totally sure. (It's possible I just attributed it to the issue with being unable to get the XML for typCreate-ed types.)

george moromisato 25 Feb 2017:

This never worked, unfortunately.

At load-time, we convert from XML text to CXMLElement object and then to specific design types (such as CShipClass). The path looks something like this:

"<ShipClass ...>" -> CXMLElement -> CShipClass

Before typGetXML we used to throw out the CXMLElement object after we created the CShipClass object.

Overrides work on the ship class object itself:

"<ShipClass ...>" -> CXMLElement[1] -> CShipClass[1]
"<ShipClassOverride ...>" -> CXMLElement[2] -> CShipClass[2]

CShipClass[1] + CShipClass[2] -> CShipClass[3]

In other words, we load <ShipClassOverride> as a separate CShipClass object and then combine it with the original ship class to create a third CShipClass object. Because of this, there is no XML for the override.

To fix this we probably need to re-implement the override concept in terms of XML. We need to do something like this:

"<ShipClass ...>" -> CXMLElement[1] -> CShipClass[1]
"<ShipClassOverride ...>" -> CXMLElement[2] -> CShipClass[2]

CXMLElement[1] + CXMLElement[2] -> CXMLElement[3] -> CShipClass[3]

The advantage of this technique (apart from fixing this bug) is that we might be able to combine two CXMLElement objects generically, regardless of design type. Right now, we need to write separate code for overriding ship classes vs. station types vs. system types. But if we do the override at the XML level, we don't have to write special code for each design type (since we already know how to convert from XML to a design type).

Of course, this would force us to always save XML, but we're heading that way anyways.

nms 26 Feb 2017:

To fix this we probably need to re-implement the override concept in terms of XML... The advantage of this technique (apart from fixing this bug) is that we might be able to combine two CXMLElement objects generically, regardless of design type.
I think this is a good idea. It seems like it would also make it easier for a type to inherit from another arbitrary type.

However, it's not quite as simple as it seems. For each element that has a corresponding element in the type that's being overridden or inherited from, you have to decide whether to add in addition to it, totally replace it, or merge with it. If you're merging, you have to decide whether to replace all the attributes, or only the ones that are specified. And maybe you want to specify this behavior for your subelements. A reasonable default might be for top-level subelements of a type to merge, replacing only specified attributes, and for their subelements to be added in addition to existing ones, but there should probably be attributes that can be set to control this.

Of course, this would force us to always save XML, but we're heading that way anyways.
I don't see why you couldn't continue to discard the CXML once all types are loaded if you're not using mods that read XML, but I also think it's not a big problem to save it.

george moromisato 26 Feb 2017:

I don't see why you couldn't continue to discard the CXML once all types are loaded if you're not using mods that read XML, but I also think it's not a big problem to save it.

The problem is that different saved games could have different overrides, so you have to do the CXMLElement merging at bind-time (not load-time).

For example, imagine extension A overrides ship class 1. In SotP, ship class 1 might have a certain definition. But in Eternity Port, it could have a different definition (this was true of Huari ships at one point). That means that extension A on SotP would behave differently than extension A on Eternity Port.

Which means we can't do the extension A override until we actually load the game and see what extensions it's using. At that point, we'll need the original XML.

george moromisato 26 Feb 2017:

However, it's not quite as simple as it seems.

Yeah, you're right. I hadn't thought it through to the next level. Still, essentially we need a set of flags per content element.

nms 26 Feb 2017:

The problem is that different saved games could have different overrides, so you have to do the CXMLElement merging at bind-time (not load-time).

I see, so you'd have to modify the engine to redo the binding when changing adventure/extension sets, which would increase load times. That's probably not worth it for the memory you save.

Still, essentially we need a set of flags per content element.

Yes, but preferably in a human-readable form. Also, I realized in some cases (e.g. <Communications><Message>) you might want to replace a corresponding element only if it has the same attributes, and add otherwise. This is probably the best default for elements below the top level subelements. Maybe something like:

overrideMode="add|replace|replaceMatching|merge|mergeReplacingAttributes"
subelementOverrideMode= ...

Priority would be:
1) element's overrideMode
2) parent element's subelementOverrideMode
3) default

nms 20 May 2018:

With the improvements to inheritance, is it possible to fix this now? It's still a problem.

george moromisato 15 Jan 2019:

In 1.8 Beta 5 I've changed ShipClassOverride to use the new inheritance improvements. This should fix the problem.