Enhancement of the XML connection

[1337]

Very, very interesting thoughts. I would like to make a few comments and additions.

Generally I very much like your constraints. I always feel strongly about clearly defining how things should work and otherwise not allow it all. That way we prevent a lot of unforeseen problems.

[*]All XML attributes should be changeable after the object was created and also after the object was ticked. If we build an editor, the level creators will play with the values and they should see the result.

I might have to disagree here. In some situations it might be almost impossible to change an attribute. Or it might involve a massive workaround that nobody uses after all.
As much as I like the concept, sometimes it's just not worth writing 500 LOC for a feature that isn't useful at all, just because our guidelines say so and the editor would like to have it. In the past, I know I've written a lot of such code just because I thought it made the concepts look good. But in the end, I could have saved a lot of time by deeming something of little use as impossible.
In rare occasions, a reload might be unavoidable.

=> No complex stuff in constructors/destructors. In particular no virtual functions.

I agree, but it doesn't work well with the "new should be enough" constraint. So sometimes a full object creation may (and possibly will, also by careless use) involve a lot of additional steps that may even (but shouldn't) modify the object itself.
I think we somehow should have a way to prevent this.

In general about order: I think it should be possible to (implicitly) define dependencies (and I believe that's where you're heading anyway) after all. But in order to enforce the concept, the default behavior should be to randomly change the order every time you load a level! I know that might generate additional problems for PPS students, but we will get lazy otherwise.
The same goes for attributes.

So I think this gives an impression of my general mindset, but it's way too early to make any final conclusions. In my opinion, creating an object just by calling "new" should be enough. After that, the object is in a neutral state where it doesn't crash and can be modified at any later time.

That would be a very good concept just in the C++ way. We might want to make more use of Exceptions then because that's the only way to abort a destructor.

Another thought about level loading might then be: Just using "new" should be enough, but of course it might be that the object has to do more computations in the first tick. As an example, take an object that does additional analysis every 10s. Having many of them, these analysis can be evenly distributed after some time by using slight randomization in update time. But at the very beginning after loading, we might have a very long first tick, making the dt in the second tick considerably large. That could already lead some unexpected behavior.
My suggestion is that any object should be perfectly comfortable with tick(0), even if the time factor is not 0. And then the object should take these 0s literally.
Currently I'm not even sure whether we have any objects that don't adhere to these rules though.
That would also allow objects to examine its surrounds, something that is not yet possible at load time.

Just one question:

An object should never have config values and an XMLPort at the same time

I don't undestand this one. Why is that a problem?

Confusion. A game object should be configurable by XML only, whereas a control object should only have config values.
This applies 99% at the moment I believe, but I would enforce it.

[x3n]

[*]All XML attributes should be changeable after the object was created and also after the object was ticked. If we build an editor, the level creators will play with the values and they should see the result.

I might have to disagree here. In some situations it might be almost impossible to change an attribute. Or it might involve a massive workaround that nobody uses after all.
As much as I like the concept, sometimes it's just not worth writing 500 LOC for a feature that isn't useful at all, just because our guidelines say so and the editor would like to have it. In the past, I know I've written a lot of such code just because I thought it made the concepts look good. But in the end, I could have saved a lot of time by deeming something of little use as impossible.
In rare occasions, a reload might be unavoidable.

Sure, it makes no sense to enforce a concept if it breaks two others. But I think in most cases it's not too hard to make an attribute changeable. Look at the model class: It wraps a mesh object. If you change the mesh-source of the model, it could simply create a new mesh. We're already forced to separate creation and loading of an object (because of xml loading and network synchronization), so why shouldn't it be possible to perform the loading process twice for the same object? I general this shouldn't add too much additional lines of code. If it does, well, then we make an exception of the rule. If this turns out to be a real pain in the editor, then the 500 LOC might be worth it.

=> No complex stuff in constructors/destructors. In particular no virtual functions.

I agree, but it doesn't work well with the "new should be enough" constraint. So sometimes a full object creation may (and possibly will, also by careless use) involve a lot of additional steps that may even (but shouldn't) modify the object itself.
I think we somehow should have a way to prevent this.

I currently can't think of an object which needs a postCreation() function. I know about one or two which need a preDestroy() function which is currently "solved" by using destroy() instead of delete. It's not too hard to change this code though, so I don't think this is a real issue in our framework. I mean, you're right, we should prevent this, but this is more of a coding guideline than a framework design decision.

My suggestion is that any object should be perfectly comfortable with tick(0), even if the time factor is not 0. And then the object should take these 0s literally.
Currently I'm not even sure whether we have any objects that don't adhere to these rules though.
That would also allow objects to examine its surrounds, something that is not yet possible at load time.

Agreed. I don't really understand the point about examining its surrounds though. How is this related to tick()? You mean after loading you call tick(0) before the game actually starts? The problem with that is that objects are also created at a later time (e.g. projectiles). To avoid the loading delay we'll need background loading and/or a smart pre-loading mechanism.

Also I don't think an object should be designed in a way which requires examining the "world" once and then operate on this knowledge. Our world is dynamic by design (it's not a simulation).

Just one question:

An object should never have config values and an XMLPort at the same time

I don't undestand this one. Why is that a problem?

Confusion. A game object should be configurable by XML only, whereas a control object should only have config values.
This applies 99% at the moment I believe, but I would enforce it.

Agreed. Config values can be understood as static values, except that they are not technically static. Hence there shouldn't be more than one instance of a configurable class at a time (except for some rare cases). XMLPort on the other hand is targeted to create lots and lots of instances of the same class, but each with different values.

However I don't see the need to enforce a separation of these two configuration methods. For example we might want to define overlays in XML, but have a config value for the font-size. Although, according to my statement above, this config value would probably rather belong to the graphics manager class. But maybe we want to make the graphics manager xml-loadable (instead of using ogre.conf).

Of course you shouldn't make the same variable configurable through a config file and and an xml file, but this is obvious because you shouldn't modify a config value in c++ anyway (except if you intentionally call ModifyConfigValue which changes both, the value and the config file).

[x3n]

Apart from thinking about design guidelines and generic concepts, I of course also start planning some implementation details.

XMLPort() and parsing an XML-element
Right now XMLPort is a virtual function. If an XML-element is parsed, it is passed to the XMLPort function where the different macros extract whatever they need (attributes, sub-elements). Each class passes the XML-element to its base class until it reaches BaseObject, the base of all XML-loadable classes.

As explained in one of the very first posts, this has some disadvantages, because it's completely distributed - no XMLPortParam macro knows about the other macros. For example it's impossible to tell if the XML element contains attributes which are not defined in XMLPort (if you write "positon" instead of "position", you will not get an error or warning, instead the loading silently succeeds and the object will always be placed at (0, 0, 0)).

Also it's not possible to overwrite the behavior of XMLPort in a subclass, e.g. if you want to change a default value (note that default values are currently under discussion because they may lead to inconsistency, see the posts above).

So I'd like to collect all information about XMLPort at startup, much like Identifiers or config values are created (e.g. by adding it to the RegisterObject macro). This means XMLPort doesn't have to be a virtual function anymore and it is not used to actually parse the XML element. It just defines the allowed XML attributes and the corresponding functions to get and set the values. From this information we can build a list of attributes for each class.

If an XML-element is loaded, the loader retrieves this list and calls the registered functions. It can also check for undefined attributes and print a warning. From this list we can also create XML validation (if required) or an editor (definitely required).


XMLPortable interface
Currently all XML loadable classes have to inherit from BaseObject. But we already had a case where we wanted to load some attributes of an interface (was it RadarViewable?) through XML. Unfortunately that's not possible. With the re-design of the XML framework I want to change this. XML loadable classes should inherit from an interface. Not sure about the name yet, XMLPortable would be an obvious choice but sounds a bit silly.


XMLPort and Lua
Now that we have an interface and a static list of all attributes for each class, it makes sense to add a new way to access attributes of an object. How about this:

Code: Select all

XMLPortable* station = new Model();

station->xml["mesh"] = "space_station.mesh";
station->xml("attached").push_back(new Billboard());
station->xml("attached").push_back(new Billboard());
station->xml("attached")("Billboard")[0]["color"] = ColourValue::Red;
station->xml("attached")("Billboard")[1]["color"] = ColourValue::Green;
station->xml["position"] = Vector3(100, 0, 0);

This would be equal to this XML code:

Code: Select all

<Model mesh="space_station.mesh" position="100, 0, 0">
  <attached>
    <Billboard color="1,0,0,0" />
    <Billboard color="0,1,0,0" />
  </attached>
</Model>

Well I have to admit this is not very useful in C++. But it gets a lot more useful if you export this to Lua. Writing scripts that create and modify objects has never been easier before. And we don't have to export all classes and functions to Lua to make this possible. XMLPort and a few helper classes (vector for sub-elements, factory to create objects) should be enough.

[1337]

Sure, it makes no sense to enforce a concept if it breaks two others. But I think in most cases it's not too hard to make an attribute changeable. Look at the model class: It wraps a mesh object. If you change the mesh-source of the model, it could simply create a new mesh. We're already forced to separate creation and loading of an object (because of xml loading and network synchronization), so why shouldn't it be possible to perform the loading process twice for the same object? I general this shouldn't add too much additional lines of code. If it does, well, then we make an exception of the rule. If this turns out to be a real pain in the editor, then the 500 LOC might be worth it.

Since this might be important, I've ha a look into a lot of our XMLPort() functions. Mostly, we already have the desired behaviour and if not, the fix is often very easy. So me concerns don't seem to apply widely to XML (I was probably thinking a lot about moving code to the c'tor to reduce flexibility and greatly reduce coding time).
However there is still a lot of work to be done to correctly fix all issues. A few examples:

• Changing a pawn's health might kill it instantly
• What happens if you change a pawn's initial health if it has already been created but the match hasn't started yet? I guess we don't have that situation, but it only serves as a possible example.
• Changing the number of explosion junks for a space ship with instant effect is probably where we have to draw the line ^^
• CameraPosition has "mouselook" XML parameter that is only queried by another object. So changing it can not possibly have the desired effect without rewriting some code.
• There are more such parameters in CameraPosition because this class acts more like a struct, so it just provides a few parameters. These might be problematic.
• Changing the position of an object with static collision shape will probably render physics inert, meaning that the object has to be completely recreated.
• Similar problem for changing the collision type
• The "hasPhysics" attribute of the Scene probably has to be redesigned.
• What happens if you change the Gametype of a Level?
• What happens if you change a pickup from "use many times" to "use once" if it has already been used? The answer is probably simple, but we still have to think about it.
• Change "maxSpawnedItems" in PickupSpawner if there are already more pickups out there.
• Try to change the resourceGroup of a ResourceCollection...
• Almost all the XML attributes in ResourceLocation and ResourceCollection exhibit problems. Since they're not in any way connected to a level, we probably have to find a different solution. (more comments on that in my next post to your latest answer)

Almost all of these items (and the list is only an excerpt) have a rather simple solution. But nevertheless, it has to be implemented, dozens of times.
I hope you see my concerns. I'm definitely a great fan of your concept, I just see problems implementing it.

I currently can't think of an object which needs a postCreation() function. I know about one or two which need a preDestroy() function which is currently "solved" by using destroy() instead of delete. It's not too hard to change this code though, so I don't think this is a real issue in our framework. I mean, you're right, we should prevent this, but this is more of a coding guideline than a framework design decision.

I'm not thinking about postCreation() functions, just about the constructor. But I see the technical problems... So yeah, a guidline has to do I think.

I don't really understand the point about examining its surrounds though. How is this related to tick()? You mean after loading you call tick(0) before the game actually starts? The problem with that is that objects are also created at a later time (e.g. projectiles). To avoid the loading delay we'll need background loading and/or a smart pre-loading mechanism.

I'm not talking about something that HAS to be done. But it might prove useful for an object to be ticked once before the game starts, but without a delta time. Of course everything still has to work without it. But maybe your smart pre-loading mechanism already takes care of that.

Also I don't think an object should be designed in a way which requires examining the "world" once and then operate on this knowledge. Our world is dynamic by design (it's not a simulation).

Sorry, didn't clarify: That would be stupid for course
Imagine some self organising AI (we might even have this already) that examines its surroundings all the time. But that doesn't change too much in one tick (the positions of objects may always change for course), so maybe a ship gets destroyed or spawns, not 100. But at the beginning of the game, many objects may need additional computation (like adding to a list). This doesn't apply for pawns for course. So I'm not sure we have this.
But considering that this is a 1 minute task to do (just tick(0) every object before starting the game), we should definitely give i a thought.

However I don't see the need to enforce a separation of these two configuration methods. For example we might want to define overlays in XML, but have a config value for the font-size.

Agreed.

Although, according to my statement above, this config value would probably rather belong to the graphics manager class. But maybe we want to make the graphics manager xml-loadable (instead of using ogre.conf).

Please explain, what exactly would be your motivation? I can see reasons too.

[x3n]

Thanks for taking your time to dig through the XMLPort functions. Your list of potential problems is quite insightful. I have to admit that I didn't think the guidelines all through, so after reading your post I think we have to make some assumptions about what's useful and what not.

When I say XML attributes should be changeable, I don't mean that you have to rollback all actions which depend on the old value of the attribute. It's perfectly ok to assume the object was just created with this attribute and never was in an other state before.

For example for the pickup this means if you change it to "use once" you don't care if it was already taken before. It simply means the next time someone picks it up it won't respawn.

Or another example, you asked "What happens if you change the Gametype of a Level?". The gametype attribute of the level object is only relevant at one point during the lifetime of a level: at the time when the level manager decides to start the level (note: start != create). If the gametype attribute is changed at a later point, we won't restart the level.

To fullfill the condition of being changeable, it's sufficient if - at the point when the level is started - the gametype is defined by the last assigned value. Example:

Code: Select all

Level* level = new Level();
level->xml["gametype"] = "Deathmatch";
level->xml["gametype"] = "CaptureTheFlag";

level->start();

If, after executing this code, the level creates a gametype object of type "CaptureTheFlag", the gametype attribute is correctly implemented to be changeable. However if the gametype remains "Deathmatch", then the attribute is not changeable because the first assigned value remains active.

The same holds for attributes related to the physics engine. At the point when the object "enters" the "physics simulation", changes of some attributes will not take effect anymore.

At the first glance this may look like a serious restriction of the general "changeable" policy. But on the other hand a lot of this will not be important in the editor - because in the editor we will not start a level and we probably will not use physics. This also means that camera settings are no problem and spaceships of course don't explode.

On the other hand, if we write Lua scripts which call XMLPort functions during an active level, some of these restrictions will be more important. But I think in this case it's ok if we write a comment "can not be changed after xxxx" or "change only takes effect if yyyy". Of course if you change an attribute it shouln't have unwanted side effects, e.g. it shouldn't kill a pawn if you change it's health.

So to summarize this, the "changeable" guideline applies mostly for attributes which are visible (or otherwise noticeable) in the editor. For all other attributes it's nice to have, but not required. For attributes which are only queried once by another class (gametype, physics, ...) it's sufficient if the last change up to this point takes effect.

For the ResourceCollection, I think we can exclude objects which don't belong to a level from this rule.

tick(0)

Ok I understand your point. I agree that tick(0) should be possible.

Although, according to my statement above, this config value would probably rather belong to the graphics manager class. But maybe we want to make the graphics manager xml-loadable (instead of using ogre.conf).

Please explain, what exactly would be your motivation? I can see reasons too.

Actually there is no real motivation. I simply got a little lost with my example of a combination of XMLPort and config values. Since GraphicsManager is a singleton, it makes more sense to load it's values from the config file. And if there is a config file, there is little need for an additional xml file.

But maybe at some point we will have such a combination with gametypes. Some attributes of a gametype may depend on the level (e.g. the number of teams in a capture the flag level - you could build a map for 3 teams) while other attributes depend on the preference of the player (e.g. the number of bots)

[1337]

I like your ideas about the implementation details. That sounds like a massive improvement.

Since everything basically needs to be rewritten and redesigned from scratch, I have one suggestion, but without an actual implementation.
I'm talking about ResourceCollection and ResourceLocation. These are actually just XML loaded config values and that poses some problems. Nevertheless, the object oriented structure of XML makes it much more useful for these two classes than actual configValues.
My suggestion is to somehow separate XML loading from level loading so that we can use the same XML loading mechanisms for pretty much anything, but don't have to be bound by some level loading restrictions.
Now, when I read your implementation ideas, this might come in naturally anyway, but I wanted to mention it.

XMLPortable interface
Currently all XML loadable classes have to inherit from BaseObject. But we already had a case where we wanted to load some attributes of an interface (was it RadarViewable?) through XML. Unfortunately that's not possible. With the re-design of the XML framework I want to change this. XML loadable classes should inherit from an interface. Not sure about the name yet, XMLPortable would be an obvious choice but sounds a bit silly.

There might be an implementation issue here: AFAIK an Orxonox class cannot inherit from the same interface twice, but please correct me if I'm wrong.
But I guess this might be easy to fix.

Another reference case for the need of an XML interface might be the sound framework. Please have a look at these too because BaseSound is designed like an interface and has XML attributes.

As for the name, I'm coming up short too for good names. What about XMLLoadable?

One more thing about the previous post:

Of course if you change an attribute it shouln't have unwanted side effects, e.g. it shouldn't kill a pawn if you change it's health.

I disagree, but not in this exact case because changing the health doesn't create a paradox (mistake on my part). However, I strongly think that changing an XML attribute should create as few paradoxes as possible. As a cheap example, imagine a "sphere of death": if you get within its radius, you die immediately. Increasing that radius should then immediately kill the pawns within the new radius.
Ok, bad example because the tick will take care of that anyway. But I think you get the point.
Gametype is also such a paradox, but pretty much unavoidable. Well, it could be avoided by renaming the XML attribute to something like "gametypeOnLevelStart", which is stupid. But I now get your point

[x3n]

My suggestion is to somehow separate XML loading from level loading so that we can use the same XML loading mechanisms for pretty much anything, but don't have to be bound by some level loading restrictions.

Yes this is a good point and I also want to implement it this way. I guess something like this sould be fine:

Code: Select all

XMLPortable* load(xml);

or maybe this way:

Code: Select all

std::vector<XMLPortable*> load(xml);

It depends on whether we allow only one root node in the XML file or multiple nodes.

There might be an implementation issue here: AFAIK an Orxonox class cannot inherit from the same interface twice, but please correct me if I'm wrong.
But I guess this might be easy to fix.

Could you elaborate a bit? What exactly is the problem?

What about XMLLoadable?

Yes that was also my first thought, but since it's also xml saveable this name is maybe a bit confusing.

As a cheap example, imagine a "sphere of death": if you get within its radius, you die immediately. Increasing that radius should then immediately kill the pawns within the new radius.

I think this example shows very well how difficult this topic ist. There are two typical ways to implement this: a) iterating through all pawns in tick() b) using a ghost object and callbacks from the physics engine.
In a) the xml attribute of the radius will be queried every tick - so in my opinion you don't have to instantly kill all pawns within the setRadius() function, it's ok to wait for the next tick. In b) you'd have to change the collision shape which may take 1 or 2 ticks of the physics engine until you finally get your callbacks. There you have no possibility to instantly kill the pawns anyway.

I'm not sure if this relaxation of the rule is confusing or not, but I think it makes sense. The changes don't necessarily have to immediately take effect, it's just important that they do as soon as possible (ideally the next time you can notice it, i.e. before the next tick of the graphics renderer).

In one of my previous posts I wrote that the changes shouldn't depend on tick() because it's not clear that we'll have a tick in the editor. So this looks like a contradiction, but on the other hand since we have no tick in the editor, a "sphere of death" has no effect anyway.

I don't know how you see this, but in my opinion the rules "attributes should be changeable" and "changes should not depend on tick()" can be relaxed in some cases:

- xml attributes which are visible or otherwise noticeable (e.g. sound) in the editor should change immediately
- changes of xml attributes which are only used in tick() can be delayed until the next tick() (or show no effect at all in the editor). you can update it earlier though (like killing all pawns within the new radius of the sphere), but I'd say it's optional
- attributes which are only queried in some occasions don't have to take effect at all since it would require a complete rollback which is not feasible
- attributes which change the physics are probably not required to take effect in the editor because I think we'll have no physics there. in the game it depends on whether bullet allows this change without much work. otherwise we simply prohibit it

[1337]

I like those rules at the end of your post. These should server as a guide line I guess.

As for the sphere of death and tick(): that's exactly what I meant with "bad example" ^^

It depends on whether we allow only one root node in the XML file or multiple nodes.

If I remember correctly, XML only allows one root node anyway. So if we want multiple, we'd require a workaround that would probably require a class like "XMLCollection" or whatever that can group any number of objects into one single root node.
Anyway, I deem it very good idea to be able to directly return the content upon loading. That makes XML loading more versatile.

@Multiple interface inheritance:
Lets take WorldSound as example: Both base classes, BaseSound as well as StaticEntity would inherit XMLPortable. That creates approximately the following situtation:

Code: Select all

struct Foo : public Tickable
{
    Foo() { RegisterObject(Foo); }
};
struct Bar : public Tickable
{
    Bar() { RegisterObject(Bar); }
};
struct Asdf : public Foo, public Bar
{
    Asdf() { RegisterObject(Asdf); }
};

That particular piece of code doesn't compile because of some "ambiguous conversion" in

Code: Select all

SUPER_FUNCTION(1, Tickable, tick, true);

in Tickable.h.

That's one issue I've just encountered. What I really meant was more general: it took you quite some effort to make OrxonoxClass the base class for interfaces and normal classes. In the case of XMLPortable, every class implementing it would probably have to do a virtual inheritance plus it might require some adjustments in createClassHierarchy() to take that into account.
I'm not at all saying it is impossible, but it is something that has to be dealt with.

[x3n]

If I remember correctly, XML only allows one root node anyway. So if we want multiple, we'd require a workaround that would probably require a class like "XMLCollection" or whatever that can group any number of objects into one single root node.

Yes, but most parsers allow it. They usually use an anonymous super-root node. I'm not sure how the standard is defined, we have to check that. For level files we'll have only one root node anyway, but we could easily define multiple templates in the same file (similar to ogre's material files). We will probably support some helper classes for pimitive values and stl containers, so you could use a vector directly instead of "XMLCollection" if this is required.

@Multiple interface inheritance:
[...]

Ok I see what you mean. However there's no general problem with OrxonoxClass and multiple inheritance, there's only an obvious problem with SUPER which of course doesn't work with multiple inheritance.

The new XMLPort doesn't have to be virtual anymore and will be executed only once at startup to collect the information, so it doesn't require SUPER. You're right about the virtual inheritance though, but there are probably not many classes inheriting directly from this interface.

[1337]

I checked to see whether the class hierarchy would support this and it does if you use RegisterRootObject() for all classes that don't inherit BaseObject, but only OrxonoxClass.