C++ skills & bored? Help us. (junior job :-))

[Tobi]

Ok, everything of today is committed including your patches malric (tho slightly modified as __VA_ARGS__ isn't in C++ standard according to wikipedia and I know from experience that it doesn't work in MSVC 6/7).

Have fun in attacking the remaining classes

[jcnossen]

I added CReadMap, CSm3ReadMap and CSmfReadMap registration.

For the actual savegame stuff, I was thinking that the easiest way is probably to write a memberless CGameSerializer class, register a serializer function to it, and call serializer.SerializeObjectInstance on all those global handler classes (readmap, uh, ph, featureHandler....). That way you can serialize only the synced parts of those, and maintain the original created objects.

[malric]

Do you agree with copying your changes to osrts as well? (LGPL)

Of course.

@Tobi

The remaining classes = rts/Sim/Path (Tobi said will work on these), rts/Sim/Projectiles (I'll start and probably finish these today...) and rts/Sim/Units ?

[Tobi]

Of the stuff in rts/Sim, yeah. I think many of the Unit and Projectile classes (for explosion generator) are already in creg tho, but a check can't hurt, there are probably some missing...

After rts/Sim probably (part of) rts/ExternalAI needs to be done too. Jelmer already did rts/Map so I think thats fine too.

For the actual savegame stuff, I was thinking that the easiest way is probably to write a memberless CGameSerializer class, register a serializer function to it, and call serializer.SerializeObjectInstance on all those global handler classes (readmap, uh, ph, featureHandler....). That way you can serialize only the synced parts of those, and maintain the original created objects.

Sounds like a good solution.

[malric]

I have seen code like:

Code: Select all

CR_REG_METADATA(CBitmapMuzzleFlame,
(
        CR_MEMBER_BEGINFLAG(CM_Config),
                CR_MEMBER(sideTexture),
                ....
                CR_MEMBER(frontOffset),
        CR_MEMBER_ENDFLAG(CM_Config)
));

What is with the flag CM_Config ? I see it is used just in ExplosionGenerator.cpp but the code wasn't obvious.

[jcnossen]

It marks that the property is configurable, as opposed to a cache value/calculated from other properties.

[malric]

So it is mandatory to be set for the weapon projectile classes ?

[jcnossen]

The flag is only used by the explosion generator, the creg serializer ignores it. The weapon projectile classes (Those that derive from CWeaponProjectile) can't be used by the explosion generator, so those don't need CM_Config.

Making the other classes work in the explosion generator usually requires a little checking too, so just do all new stuff without CM_Config...

[jcnossen]

So what's left is..

- CommandAI
- Pathfinder
- GlobalSynced
- COB
- other global synced data?
- actual saving/loading code
- networking support for rsyncing savegames within a game

[Tobi]

Yeah, sort of. Part of pathfinder I've done locally here already, but I wasn't in the mood for coding today so I didn't do really much.

A lot of time will be in bugfixing/testing loading code I fear.

Also it would be a waste to delete the entire game state only to recreate it by deserialization immediately after, so I think there must be a method designed to reserialize an object (ie. assume things have already been allocated and only fill in the primitive types or something.. tho it should probably handle e.g. a missing unit too).

Anyway, I'll make some rsync code soonish..

[jcnossen]

Also it would be a waste to delete the entire game state only to recreate it by deserialization immediately after, so I think there must be a method designed to reserialize an object (ie. assume things have already been allocated and only fill in the primitive types or something.. tho it should probably handle e.g. a missing unit too).

IMO, we should try the simple case first, such an optimization probably requires quite a lot of additional work.

[jcnossen]

A lot of time will be in bugfixing/testing loading code I fear.

Actually we don't really have to save everything, as long as we save an acceptable version of the current game, and have it synced. With that i mean that we could ignore certain parts if they are a lot of work (such as CommandAI).

I'm going to add some actual load/save code now

[Tobi]

Yeah true (regarding that optimization).

Also true regarding this part of the game saving - as long as it doesn't affect gameplay too much (no erasing of commandQue's ) and the loading is deterministic it should be reasonably fine...

[malric]

Question. In rts/Sim/Misc/LosHandler.cpp there is something like:

Code: Select all

void CLosHandler::creg_Serialize(creg::ISerializer& s)
{
	// NOTE This could be tricky if gs is serialized after losHandler.
	for (int a = 0; a < gs->activeAllyTeams; ++a) {
		s.Serialize(losMap, losSizeX*losSizeY*2);
		s.Serialize(airLosMap, airSizeX*airSizeY*2);
	}
}

The definitions of losMap and airLosMap are:

Code: Select all

	unsigned short* losMap[MAX_TEAMS];
	unsigned short* airLosMap[MAX_TEAMS];

(arrays of pointers).

Shouldn't the code be something like:

Code: Select all

...
	CR_MEMBER(losMap),  // to serialize the array of pointers...
	CR_MEMBER(airLosMap), // to serialize the array of pointers...
...

void CLosHandler::creg_Serialize(creg::ISerializer& s)
{
	// and now serialize the pointers content...
	// NOTE This could be tricky if gs is serialized after losHandler.
	for (int a = 0; a < gs->activeAllyTeams; ++a) {
		s.Serialize(losMap[a], losSizeX*losSizeY*2);
		s.Serialize(airLosMap[a], airSizeX*airSizeY*2);
	}
}

?

[Tobi]

Have you tried that?

It gives a compile error: creg can't know beforehand how big the arrays are, so it can't serialize them. Hence they must be serialized manually in this creg_Serialize() function.

[malric]

I see I doesn't compile but I think the reason is a bit different.

The arrays are of fixed size MAX_PLAYERS. The pointer the array contains are of an unknown size. (but yes, creg doesn't seem to handle that - so it doesn't compile)

I have looked for Serialize, and I assume the one called is the one from COutputStreamSerializer which saves 'byteSize' bytes from 'data'.

I guess then that it should be done like this:

Code: Select all

	// NOTE This could be tricky if gs is serialized after losHandler.
	s.Serialize(losMap,sizeof(unsigned short*)*MAX_PLAYERS);
	s.Serialize(airLosMap,sizeof(unsigned short*)*MAX_PLAYERS);
	for (int a = 0; a < gs->activeAllyTeams; ++a) {
		s.Serialize(losMap[a], losSizeX*losSizeY*2);
		s.Serialize(airLosMap[a], airSizeX*airSizeY*2);
	}

PS. I assume there will be a Unserialize function (for the above to work at loading also) ?

[jcnossen]

Code: Select all

   s.Serialize(losMap,sizeof(unsigned short*)*MAX_PLAYERS); 

Serializing pointers doesn't work, the address will be different the next time it is allocated.
Also, there is no Unserialize, this is also handled by Serialize. You can check for loading/saving with ISerializer::IsWriting()

[malric]

Ok, got the whole idea. Thanks.

So in CLosHandler::creg_Serialize if gs was serialized after CLosHandler then in the constructor of CLosHandler the arrays will not be created correctly...

But I still think it should be:

Code: Select all

	for (int a = 0; a < gs->activeAllyTeams; ++a) {
		s.Serialize(losMap[a], losSizeX*losSizeY*2);
		s.Serialize(airLosMap[a], airSizeX*airSizeY*2);
	}

[jcnossen]

yeah i agree.