SystemPath.h - is this code correct?

Post Reply
nick
Posts: 85
Joined: Mon Sep 08, 2014 9:24 pm
Location: Plymouth, UK

SystemPath.h - is this code correct?

Post by nick »

I came across this a couple of weeks ago, and wonder if anyone can shed any light on it:

Code: Select all

void Serialize(Serializer::Writer &wr) const {
		wr.Int32(sectorX);
		wr.Int32(sectorY);
		wr.Int32(sectorZ);
		wr.Int32(Uint32(systemIndex));
		wr.Int32(Uint32(bodyIndex));
	}
The above code saves three signed ints and two unsigned ints (which have an unnecessary cast to Uint32).
The corresponding load code is shown below:

Code: Select all

static SystemPath Unserialize(Serializer::Reader &rd) {
		Uint32 x = rd.Int32();
		Uint32 y = rd.Int32();
		Uint32 z = rd.Int32();
		Sint32 si = Sint32(rd.Int32());
		Sint32 bi = Sint32(rd.Int32());
		return SystemPath(x, y, z, si, bi);
	}
So the three signed ints are assigned to unsigned, and the two unsigned are cast to signed.
Is this by design? Won't it ensure that "sectorX/Y/Z" can never be negative upon loading?
[Edit]Or is this clever use of casting: I think casting -1 to unsigned will lead to the largest possible signed?
robn
Posts: 302
Joined: Mon Jul 01, 2013 1:11 am
Location: Melbourne, Australia

Re: SystemPath.h - is this code correct?

Post by robn »

Sector x,y & z are signed, system and body indices are unsigned. The serializer only has methods for unsigned ints, so we cast.
nick
Posts: 85
Joined: Mon Sep 08, 2014 9:24 pm
Location: Plymouth, UK

Re: SystemPath.h - is this code correct?

Post by nick »

Thanks robn :)
With the JSON save/load, we can handle both signed and unsigned.
nick
Posts: 85
Joined: Mon Sep 08, 2014 9:24 pm
Location: Plymouth, UK

Re: SystemPath.h - is this code correct?

Post by nick »

Regarding the existing serialisation of ints:

I've always assumed that the bit pattern of either signed or unsigned is saved, and it'll always work - so long as you cast the saved binary data back to the correct type (signed or unsigned). So I 've been checking the exact type of each int variable that has been serialised, and using the appropriate JSON one.
robn
Posts: 302
Joined: Mon Jul 01, 2013 1:11 am
Location: Melbourne, Australia

Re: SystemPath.h - is this code correct?

Post by robn »

You mean Value::asInt and Value::asUint? Yes, you can use those without casting.
nick
Posts: 85
Joined: Mon Sep 08, 2014 9:24 pm
Location: Plymouth, UK

Re: SystemPath.h - is this code correct?

Post by nick »

Thanks for your support this eve :)
I now believe all the JSON save/load code is in place (I thought I was missing some data to be saved).
So now I'm looking for bugs...
Takes pioneer about 15-20 minutes to start (a debug mode thing?) and frame rate is 3 per second, which makes testing difficult. But it does force me to really think about the code before I commit to a test :)
I'd like to work on builds later than 3rd Jan, which have the openGL optimisations, but to do that I'll have to merge, so for now I'm stuck with 3rd Jan.
When I fix the current issues, I'll still have a lot more testing to do, but at that point I'll learn how to merge and update the PR so others can test if they want.
FluffyFreak
Posts: 1343
Joined: Tue Jul 02, 2013 1:49 pm
Location: Beeston, Nottinghamshire, GB
Contact:

Re: SystemPath.h - is this code correct?

Post by FluffyFreak »

To update your branch:
git fetch --all -p
git checkout jsonsaveload
git pull origin jsonsaveload
git pull upstream master
Resolve any merge conflicts and then:
git push origin jsonsaveload
To push the merged version back up to GitHub and it will update your PR.

Now then, performance.
Are you running on Windows using Visual Studio 2013? In that case there are four build options:
  • Debug - full-on deep paranoia checking of everything with full debug information and no optimisation at all
  • PreRelease - full debug as above but with some optimisations
  • Profile - same as below but with intrusive CPU performance profiling enabled
  • Release - full optimisations, most debugging facilities compromised or disabled, no asserts, go faster stripes
Debug is extremely slow but as you can imagine it is checking everything, you can often just use PreRelease for debugging and testing purposes.
If I am actually tracking down a specific problem and know roughly the area it is in then I often run the game in Release but I place a line like:

Code: Select all

#pragma optimize("",off)
above the function/method that I want to debug. That almost always allows breakpoints to work and give you the local/global/member variables within that methods scope.
You might have to use that for a few dozen functions/methods if something is very complex but the rest of the game will be running at full speed so testing can be far less time consuming.

It can take a couple of minutes for Pioneer to start in a Debug build. but never 15 to 20! Well not for me anyway :)
Post Reply