Source Code and Ideas question

[Pleb]

I actually thought about doing this last night, a conditional if statement could be added that if the return was a nil value, then it would revert to the original code. This was why I had the status and rating entries it looks for set to different keys than before in descriptions.plist so that the original entries would be intact, and any older OXP or OXZ that uses them for lookup is not broken. I will put something together later when I get home as I'm at work at the moment.

[Pleb]

Okay I think this will fix the nil value error. Open up OOConstToString.m again, go to line 470 and replace that section with this:

Code: Select all

NSString *OODisplayRatingStringFromKillCount(unsigned kills)
{
	NSArray			*ratingNames = nil;
	NSArray			*ratingKills = nil;
	unsigned		i;
	BOOL			newRatingsInvalid;

	ratingNames = [[UNIVERSE descriptions] oo_arrayForKey:@"rating_names"];
	ratingKills = [[UNIVERSE descriptions] oo_arrayForKey:@"rating_kills"];
	for (i = 0; i < [ratingKills count]; ++i)
	{
		if (kills < [ratingKills oo_unsignedIntAtIndex:i])  
		{
			if ([ratingNames oo_stringAtIndex:i-1] != nil)
			{
				return [ratingNames oo_stringAtIndex:i-1];
			}
			else
			{
				newRatingsInvalid = YES;
			}
		}
	}
	
	if ([ratingNames oo_stringAtIndex:[ratingKills count] - 1] == nil) newRatingsInvalid = YES;
	
	if (newRatingsInvalid)
	{
		enum { kRatingCount = 9 };
	
		const unsigned		killThresholds[kRatingCount - 1] =
							{
								0x0008,
								0x0010,
								0x0020,
								0x0040,
								0x0080,
								0x0200,
								0x0A00,
								0x1900
							};
	
		ratingNames = [[UNIVERSE descriptions] oo_arrayForKey:@"rating"];
		for (i = 0; i < kRatingCount - 1; ++i)
		{
			if (kills < killThresholds[i])  return [ratingNames oo_stringAtIndex:i];
		}
	
		return [ratingNames oo_stringAtIndex:kRatingCount - 1];
	}
	
	return [ratingNames oo_stringAtIndex:[ratingKills count] - 1];
}

Then go to line 531 and replace that section with this:

Code: Select all

NSString *OODisplayStringFromLegalStatus(int legalStatus)
{
	NSArray			*statusNames = nil;
	NSArray			*statusBounty = nil;
	unsigned		i;
	BOOL			newStatusInvalid;

	statusNames = [[UNIVERSE descriptions] oo_arrayForKey:@"legal_status_names"];
	statusBounty = [[UNIVERSE descriptions] oo_arrayForKey:@"legal_status_bounty"];
	for (i = 0; i < [statusBounty count]; ++i)
	{
		if (legalStatus < [statusBounty oo_intAtIndex:i])  
		{
			if ([statusNames oo_stringAtIndex:i-1] != nil)
			{
				return [statusNames oo_stringAtIndex:i-1];
			}
			else
			{
				newStatusInvalid = YES;
			}
		}
	}
	
	if ([statusNames oo_stringAtIndex:[statusBounty count] - 1] == nil) newStatusInvalid = YES;
	
	if (newStatusInvalid)
	{
		enum { kStatusCount = 3 };

		const int			statusThresholds[kStatusCount - 1] =
							{
								1,
								51
							};
	
		statusNames = [[UNIVERSE descriptions] oo_arrayForKey:@"legal_status"];
		for (i = 0; i != kStatusCount - 1; ++i)
		{
			if (legalStatus < statusThresholds[i])  return [statusNames oo_stringAtIndex:i];
		}
	
		return [statusNames oo_stringAtIndex:kStatusCount - 1];
	}
	
	return [statusNames oo_stringAtIndex:[statusBounty count] - 1];
}

Compile game and make sure you insert the same lines as before (from my previous post with code in) into the descriptions.plist file.

If anyone can break this again let me know and I will go back to the drawing board. Admittedly this could probably be tidied up a bit!

[another_commander]

I'm afraid it's already broken, Pleb. I tried it with these rating_kills and rating_names arrays:

Code: Select all

"rating_kills" = 
   (
      "0",
      "8",
      "16",
      "32",
      "64",
      "126",
      "512",
      "2560",
      "6400"
   );
   
   "rating_names" = 
   (
      "Harmlesssssss",
      "Mostly Harmlesssss",
      "Poorsss",
      "Averagesss",
      "Above Averagesss",
      "Competentssss",
      "Dangerousssss",
      "☆ Deadlyssss ☆",
      "★★★ E L I T Essss ★★★"
   );

This is just a copy of your arrays from previous page, with some Gollum-like sssss added at the end of each rank in order to be able to see which array the program is using, descriptions plist or fall-back one. Using debug console, I gave the player 6401 kills, which is expected to produce ★★★ E L I T Essss ★★★ for the rank. Unfortunately, it produces just ★★★ E L I T E ★★★, which means that we've gone to the emergency fall-back array when there is no reason to do so. The problem is that every time your loop runs, it sets newRatingsInvalid to YES, because at the first iteration [ratingNames oo_stringAtIndex:i-1] is always nil (it checks array index -1 or, more correctly since i is declared unsigned, it checks array index 4294967295 after i has gone below zero and turned into UINT_MAX) that the newRatingsInvalid variable can be used uninitialized and, if that happens, chances are that it will contain a non-zero value, therefore the code will enter the fallback part.

Also, the first and most basic check you should be doing is array length equality. You should immediately fall back to the emergency arrays if the lengths of kills and names don't match. To witness the problem, just try adding two or three more rating_kills elements between "8" and "16" and see what happens as the ranks go higher.

Keep it up, you're almost there.

[Pleb]

Agh! That is annoying, as I did a similar test to yours last night (just put numbers at the beginning instead of ssss at the end) and I thought this had worked! I will go back and look at this, and implement a check to confirm that the count of both arrays matches. Hopefully this will then fix this error!

[cim]

Code: Select all

	ratingNames = [[UNIVERSE descriptions] oo_arrayForKey:@"rating_names"];
	ratingKills = [[UNIVERSE descriptions] oo_arrayForKey:@"rating_kills"];

A minor point here: if introducing new entries to descriptions.plist that are intended to go into the core game, it's best to prefix them with "oolite_" (so "oolite_rating_names", etc. and similarly for legal status) as this significantly reduces the chance of a clash with existing OXPs that might have a key with that name already.

Yes, most of the existing keys do not follow this convention and it's too late to change them.

[Pleb]

Okay...taking all said before on board, I think (and hope) I may have cracked it this time:

Code: Select all

NSString *OODisplayRatingStringFromKillCount(unsigned kills)
{
	NSArray			*ratingNames = nil;
	NSArray			*ratingKills = nil;
	unsigned		i;
	int				newRatingsInvalid;

	ratingNames = [[UNIVERSE descriptions] oo_arrayForKey:@"oolite_rating_names"];
	ratingKills = [[UNIVERSE descriptions] oo_arrayForKey:@"oolite_rating_kills"];
	newRatingsInvalid = 0;
	
	if ([ratingNames count] != [ratingKills count]) 
	{
		newRatingsInvalid = 1;
	}
	else
	{
		newRatingsInvalid = 0;
	}
	
	if (newRatingsInvalid == 1)
	{
		enum { kRatingCount = 9 };
	
		const unsigned		killThresholds[kRatingCount - 1] =
							{
								0x0008,
								0x0010,
								0x0020,
								0x0040,
								0x0080,
								0x0200,
								0x0A00,
								0x1900
							};
	
		ratingNames = [[UNIVERSE descriptions] oo_arrayForKey:@"rating"];
		for (i = 0; i < kRatingCount - 1; ++i)
		{
			if (kills < killThresholds[i])  return [ratingNames oo_stringAtIndex:i];
		}
	
		return [ratingNames oo_stringAtIndex:kRatingCount - 1];
	}
	
	for (i = 0; i < [ratingKills count]; ++i)
	{
		if (kills < [ratingKills oo_unsignedIntAtIndex:i])  return [ratingNames oo_stringAtIndex:i-1];
	}
	
	return [ratingNames oo_stringAtIndex:[ratingKills count] - 1];
}

Code: Select all

NSString *OODisplayStringFromLegalStatus(int legalStatus)
{
	NSArray			*statusNames = nil;
	NSArray			*statusBounty = nil;
	unsigned		i;
	int				newStatusInvalid;

	statusNames = [[UNIVERSE descriptions] oo_arrayForKey:@"oolite_legal_status_names"];
	statusBounty = [[UNIVERSE descriptions] oo_arrayForKey:@"oolite_legal_status_bounty"];
	newStatusInvalid = 0;
	
	if ([statusNames count] != [statusBounty count]) 
	{
		newStatusInvalid = 1;
	}
	else
	{
		newStatusInvalid = 0;
	}
	
	if (newStatusInvalid == 1)
	{
		enum { kStatusCount = 3 };

		const int			statusThresholds[kStatusCount - 1] =
							{
								1,
								51
							};
	
		statusNames = [[UNIVERSE descriptions] oo_arrayForKey:@"legal_status"];
		for (i = 0; i != kStatusCount - 1; ++i)
		{
			if (legalStatus < statusThresholds[i])  return [statusNames oo_stringAtIndex:i];
		}
	
		return [statusNames oo_stringAtIndex:kStatusCount - 1];
	}
	
	for (i = 0; i < [statusBounty count]; ++i)
	{
		if (legalStatus < [statusBounty oo_intAtIndex:i]) return [statusNames oo_stringAtIndex:i-1];
	}
	
	return [statusNames oo_stringAtIndex:[statusBounty count] - 1];
}

Code: Select all

"oolite_rating_kills" = 
	(
		"0",
		"8",
		"16",
		"32",
		"64",
		"126",
		"512",
		"2560",
		"6400"
	);
	
	"oolite_rating_names" = 
	(
		"Harmless",
		"Mostly Harmless",
		"Poor",
		"Average",
		"Above Average",
		"Competent",
		"Dangerous",
		"☆ Deadly ☆",
		"★★★ E L I T E ★★★"
	);
	
	"oolite_legal_status_bounty" =
	(
		"0",
		"1",
		"51"
	);
	
	"oolite_legal_status_names" =
	(
		"Clean",
		"Offender",
		"Fugitive"
	);

This time it checks the length (count) of the arrays and makes sure they are the same length. I have also changed the keys to incorporate the oolite_ bit, as suggested by cim. I've tested this by making the arrays the same length and different lengths, and it seems to work this time, as if the lengths are not the same it reverts to the old version. I also tested this by creating a test OXP and added different values using these keys and this seemed to work too...

[another_commander]

Using these arrays as input...

Code: Select all

"oolite_rating_kills" =
   (
      "4",
      "8",
      "16",
      "32",
      "64",
      "126",
      "512",
      "2560",
      "6400"
   );
"oolite_legal_status_bounty" =
   (
      "1",
      "2",
      "51"
   );

... gives this as result:


Also, why was the type of newRatingsInvalid changed to int? Although int works, it is confusing for someone reading the code. BOOL is the preferred type for these yes/no type variables.

[cim]

Using these arrays as input...

Next test after that one is fixed: put the numbers in an unsorted order. (I would probably fallback rather than trying to fix in that case)

[another_commander]

Yes, the modified code should be able to handle also this case. Additionally, I think that the fallback "safe" arrays should really be both hardcoded (now only one of them is), just so that there is no chance that anyone can mess things up by toying also with the fallback arrays in descriptions in the same fashion we do now with the proposed arrays in this thread.

Oh, and any fallback to the safe internal arrays should cause a log notification to be issued, probably with a short description of the cause of fallback.

[Pleb]

Okay some issues fixed now. I had originally changed the BOOL variables to int as it was causing an issue, but this has been resolved by setting the BOOL variables to 0 before doing anything. I've also added log entries when the code fails and reverts to the hardcoded (original) ratings and legal statuses.

Code: Select all

NSString *OODisplayRatingStringFromKillCount(unsigned kills)
{
	NSArray         *ratingNames = nil;
	NSArray         *ratingKills = nil;
	unsigned		i;
	BOOL			newRatingsInvalid;

	ratingNames = [[UNIVERSE descriptions] oo_arrayForKey:@"oolite_rating_names"];
	ratingKills = [[UNIVERSE descriptions] oo_arrayForKey:@"oolite_rating_kills"];
	newRatingsInvalid = 0;

	if ([ratingNames count] != [ratingKills count]) 
	{
		newRatingsInvalid = YES;
		OOLog(@"new.ratings.error", @"'oolite_rating_kills' and 'oolite_rating_names' array size in descriptions.plist not equal - reverting to original ratings system.");
	}
	
	for (i = 0; i < [ratingKills count]; ++i)
	{
		if (kills < [ratingKills oo_unsignedIntAtIndex:i])  
		{
			if ([ratingNames oo_stringAtIndex:i-1] != nil)
			{
				return [ratingNames oo_stringAtIndex:i-1];
			}
			else
			{
				newRatingsInvalid = YES;
				OOLog(@"new.ratings.error", @"'nil' value returned when trying to display rating status - reverting to original ratings system.");
				break;
			}
		}
	}

	if (newRatingsInvalid)
	{
		enum { kRatingCount = 9 };

		const unsigned      killThresholds[kRatingCount - 1] =
							{
								0x0008,
								0x0010,
								0x0020,
								0x0040,
								0x0080,
								0x0200,
								0x0A00,
								0x1900
							};

		ratingNames = [[UNIVERSE descriptions] oo_arrayForKey:@"rating"];
		for (i = 0; i < kRatingCount - 1; ++i)
		{
			if (kills < killThresholds[i])  return [ratingNames oo_stringAtIndex:i];
		}

		return [ratingNames oo_stringAtIndex:kRatingCount - 1];
	}

	return [ratingNames oo_stringAtIndex:[ratingKills count] - 1];
}

Code: Select all

NSString *OODisplayStringFromLegalStatus(int legalStatus)
{
	NSArray         *statusNames = nil;
	NSArray         *statusBounty = nil;
	unsigned      	i;
	BOOL           	newStatusInvalid;

	statusNames = [[UNIVERSE descriptions] oo_arrayForKey:@"oolite_legal_status_names"];
	statusBounty = [[UNIVERSE descriptions] oo_arrayForKey:@"oolite_legal_status_bounty"];
	newStatusInvalid = 0;

	if ([statusNames count] != [statusBounty count]) 
	{
		newStatusInvalid = YES;
		OOLog(@"new.legal.status.error", @"'oolite_legal_status_bounty' and 'oolite_legal_status_names' array size in descriptions.plist not equal - reverting to original legal status system.");
	}
   
	for (i = 0; i < [statusBounty count]; ++i)
	{
		if (legalStatus < [statusBounty oo_intAtIndex:i]) 
		{
			
			if([statusNames oo_stringAtIndex:i-1] != nil)
			{
				return [statusNames oo_stringAtIndex:i-1];
			}
			else
			{
				newStatusInvalid = YES;
				OOLog(@"new.legal.status.error", @"'nil' value returned when trying to display legal status - reverting to original legal status system.");
				break;
			}
		}
	}

	if (newStatusInvalid)
	{
		enum { kStatusCount = 3 };

		const int         statusThresholds[kStatusCount - 1] =
							{
								1,
								51
							};

		statusNames = [[UNIVERSE descriptions] oo_arrayForKey:@"legal_status"];
		for (i = 0; i != kStatusCount - 1; ++i)
		{
			if (legalStatus < statusThresholds[i])  return [statusNames oo_stringAtIndex:i];
		}
   
		return [statusNames oo_stringAtIndex:kStatusCount - 1];
	}
   
	return [statusNames oo_stringAtIndex:[statusBounty count] - 1];
}

This now checks if it is going to throw a nil value, and if so will revert to hardcoded ratings and/or legal statuses and logs the error.

EDIT: I forgot to mention, you will need to add the following to logcontrol.plist in order for the log entries to appear:

Code: Select all

	new.ratings.error						= yes;
	new.legal.status.error					= yes;

Also thank you to cim and another_commander for finding holes in my code, I'm getting a crash course in fool-proofing and testing!

EDIT #2: Sorry one other thing, another_commander you mentioned about hardcoding the arrays, did you mean like what it does here and reverts to the original way of doing things, or by actually hardcoding the entries (Harmless, Clean, etc...)?

[another_commander]

OK good stuff, now we seem to be getting somewhere. I have not been able to make it misbehave with the standard set of tests. There are however a few observations:

- I can enter negative values in the ratings array and I get no complaints. I think we should be falling back to internal arrays in this case. This was not a problem in the original code because said array was hardcoded. I imagine the same goes also for the legal status values array.

- newRatingsInvalid / newStatusInvalid = 0; It is good that you initialize the variable (actual earlier problems were because of this lack of initialization and I am surprised the compiler didn't throw a warning about that). However, although 0 is the same as NO, as defined in GNUstep in your development environment's installation at <RootFolderOfDevelopmentEnvironment>\gcc\Msys_x2\1.0\Devlibs\gnustep1201\Local\Library\Headers\Foundation\NSObjCRuntime.h, it is much cleaner to initialize as
newRatingsIvalid / newStatusIvalid = NO;
This tells us immediately that we are dealing with a BOOL variable and is more consistent with later assignment of YES (rather than 1) that you do. It's not something that affects functionality, but it may affect readability for some people.

- The algorithm used for finding the correct rating name and legal status description assumes two things about the rating and legal status value holding arrays:
1. That their values are sorted.
2. That their first value is 0.
Now, this is not normally a problem with the way it was before, because the arrays in question are hardcoded, the algorithm is very slightly different to not require for the first value to be 0 and they are already sorted. Making them external requires that checks are made against these two conditions too, since they become available for anyone to apply changes to them.

If I change the first value of ratings to, say 4, I get the fallback to the safe array. But should it be this way? Users are not meant to know how the algorithms work and it is very easy for some OXPer to start the array with a non-zero value. So, what to do? In my opinion, in this case, rather than doing the fallback, maybe a special value could be returned? Something like "Unrated"? Not saying that this is necessarily the best idea, but I think that just reverting to the internal array with a message "nil was returned" can be confusing, because in the imaginary scenario above, the OXPer has not really done anything wrong.

Finally, the problem of sorting must be... ehm... sorted. By swapping the value of 2560 and 6400 in the rating values array, one can effectively disable the display of the rank Deadly. So either the algorithm must be somehow modified or the arrays we work on must be sorted before we start. And I mean both arrays in each case, values and descriptions, since they go hand-by-hand (which is a reason why having two arrays one with values and one with descriptions is probably not the best way to go about it). Alternatively, as suggested by cim, we can fall back to the internal arrays, but in this case the OXPers must know that their arrays must be sorted or they won't be used. Generally, I am not really in favor of applying such arbitrary restrictions to people who should not have to worry about the internals of the game. Although this last option is the easiest to implement, I would not really prefer it.

Regarding the hardcoding of the fallback arrays, I mean hardcoding the actual entries, both the one with contents

Code: Select all

0x0008,
0x0010,
0x0020,
0x0040,
0x0080,
0x0200,
0x0A00,
0x1900

currently already in the core code and the one with contents

Code: Select all

rating =
	(
		"Harmless",
		"Mostly Harmless",
		"Poor",
		"Average",
		"Above Average",
		"Competent",
		"Dangerous",
		"☆ Deadly ☆",
		"★★★ E L I T E ★★★"
	);

currently in descriptions. Same for the legal status ones, of course.

Such a seemingly simple change, so many complications, eh?

[Pleb]

Okay, the easy way:

Code: Select all

NSString *OODisplayRatingStringFromKillCount(unsigned kills)
{
	NSArray         *ratingNames = nil;
	NSArray         *ratingKills = nil;
	unsigned		i;
	BOOL			newRatingsInvalid;

	ratingNames = [[UNIVERSE descriptions] oo_arrayForKey:@"oolite_rating_names"];
	ratingKills = [[UNIVERSE descriptions] oo_arrayForKey:@"oolite_rating_kills"];
	newRatingsInvalid = NO;

	if ([ratingNames count] != [ratingKills count]) 
	{
		newRatingsInvalid = YES;
		OOLog(@"new.ratings.error", @"'oolite_rating_kills' and 'oolite_rating_names' array size in descriptions.plist not equal - reverting to original ratings system.");
	}
	
	if ([ratingKills oo_unsignedIntAtIndex:0] != 0)
	{
		newRatingsInvalid = YES;
		OOLog(@"new.ratings.error", @"'oolite_rating_kills' array in descriptions.plist does not start at 0 - reverting to original ratings system.");
	}
	
	if (!newRatingsInvalid)
	{
		for (i = 0; i < [ratingKills count]; ++i)
		{
			if (kills < [ratingKills oo_unsignedIntAtIndex:i])  
			{
				if ([ratingNames oo_stringAtIndex:i-1] != nil)
				{
					return [ratingNames oo_stringAtIndex:i-1];
				}
				else
				{
					newRatingsInvalid = YES;
					OOLog(@"new.ratings.error", @"'nil' value returned when trying to display rating status - reverting to original ratings system.");
					break;
				}
			}
		}
	}

	if (newRatingsInvalid)
	{
		enum { kRatingCount = 9 };

		const unsigned      killThresholds[kRatingCount - 1] =
							{
								0x0008,
								0x0010,
								0x0020,
								0x0040,
								0x0080,
								0x0200,
								0x0A00,
								0x1900
							};

		ratingNames = [NSArray arrayWithObjects:@"Harmless",@"Mostly Harmless",@"Poor",@"Average",@"Above Average",@"Competent",@"Dangerous",@"☆ Deadly ☆",@"★★★ E L I T E ★★★",nil];
		for (i = 0; i < kRatingCount - 1; ++i)
		{
			if (kills < killThresholds[i])  return [ratingNames oo_stringAtIndex:i];
		}

		return [ratingNames oo_stringAtIndex:kRatingCount - 1];
	}

	return [ratingNames oo_stringAtIndex:[ratingKills count] - 1];
}

Code: Select all

NSString *OODisplayStringFromLegalStatus(int legalStatus)
{
	NSArray         *statusNames = nil;
	NSArray         *statusBounty = nil;
	unsigned      	i;
	BOOL           	newStatusInvalid;

	statusNames = [[UNIVERSE descriptions] oo_arrayForKey:@"oolite_legal_status_names"];
	statusBounty = [[UNIVERSE descriptions] oo_arrayForKey:@"oolite_legal_status_bounty"];
	newStatusInvalid = NO;

	if ([statusNames count] != [statusBounty count]) 
	{
		newStatusInvalid = YES;
		OOLog(@"new.legal.status.error", @"'oolite_legal_status_bounty' and 'oolite_legal_status_names' array size in descriptions.plist not equal - reverting to original legal status system.");
	}
	
	if ([statusBounty oo_unsignedIntAtIndex:0] != 0)
	{
		newStatusInvalid = YES;
		OOLog(@"new.legal.status.error", @"'oolite_legal_status_bounty' array in descriptions.plist does not start at 0 - reverting to original ratings system.");
	}
   
	if (!newStatusInvalid)
	{
		for (i = 0; i < [statusBounty count]; ++i)
		{
			if (legalStatus < [statusBounty oo_intAtIndex:i]) 
			{
				if([statusNames oo_stringAtIndex:i-1] != nil)
				{
					return [statusNames oo_stringAtIndex:i-1];
				}
				else
				{
					newStatusInvalid = YES;
					OOLog(@"new.legal.status.error", @"'nil' value returned when trying to display legal status - reverting to original legal status system.");
					break;
				}
			}
		}
	}

	if (newStatusInvalid)
	{
		enum { kStatusCount = 3 };

		const int         statusThresholds[kStatusCount - 1] =
							{
								1,
								51
							};

		statusNames = [NSArray arrayWithObjects:@"Clean",@"Offender",@"Fugitive",nil];
		for (i = 0; i != kStatusCount - 1; ++i)
		{
			if (legalStatus < statusThresholds[i])  return [statusNames oo_stringAtIndex:i];
		}
   
		return [statusNames oo_stringAtIndex:kStatusCount - 1];
	}
   
	return [statusNames oo_stringAtIndex:[statusBounty count] - 1];
}

This now has all original values hard coded, the BOOL variables set to NO as default instead of 0, and a log entry informing the user that the arrays must start with 0 or code will revert to hard coded originals. So easy but annoying for OXP writers, as they now must remember that the kills & bounty arrays must start with 0, must be in numerical order and the corresponding entries must be in the same order as the array.

I will look into how these arrays could be sorted so that they are in numerical order, but as you've previously stated it might be better to find a different way of implementing this... For now this does work though.

[Zireael]

Wow, Pleb is much better at coding in Objective C than I am

[Pleb]

Wow, Pleb is much better at coding in Objective C than I am

No not at all, I'm a complete beginner to Objective-C, I've only ever used JavaScript and Visual Basic (and a bit of Pascal at college) before.

EDIT: Actually that's probably not entirely true, as technically I could list HTML, PHP and a tiny bit of C++ as well...but essentially a beginner!

[Pleb]

Well, in terms of sorting arrays it turns out it is a lot more difficult than one would expect. For example, I'm using the following code to sort the ratingKills array in my code:

Code: Select all

NSArray *sortedRatingKills = [ratingKills sortedArrayUsingSelector:@selector(compare:)];

However this now turns an array that looked like this:

Code: Select all

	(
		"0",
		"8",
		"16",
		"32",
		"64",
		"126",
		"512",
		"2560",
		"6400"
	);

Into this:

Code: Select all

	(
		"0",
		"126",
		"16",
		"2560",
		"32",
		"512",
		"64",
		"6400",
		"8"
	);

Which of course is not right! This also causes a problem with the other implementation of adding in new ratings and legal statuses that I've been working on. I've managed to create a dictionary that looks like this:

Code: Select all

{
	"0"    = "Harmless";
	"8"    = "Mostly Harmless";
	"16"   = "Poor";
	"32"   = "Average";
	"64"   = "Above Average";
	"126"  = "Competent";
	"512"  = "Dangerous";
	"2560" = "☆ Deadly ☆";
	"6400" = "★★★ E L I T E ★★★";
}

And this is saved as ratings.plist. By defining it in Universe.h and Universe.m I can call values from it using [UNIVERSE ratings]. To get just the keys from this dictionary and save them into an array I can use:

Code: Select all

NSArray 	*ratingKeys = [[UNIVERSE ratings] allKeys];

But keys are not loaded in order when using a dictionary. Therefore this array will need to be sorted, and if I use the example from before, I get the same order - which is wrong!

Also another problem with using a dictionary like this is OXP related. If someone wants to add new ratings (which is the whole point of this modification), then they can add new values, such as "7800" = "★★★★ A W E S O M E ★★★★"; (I'm only messing around, that's a terrible rating!), then that works. But the original values will always be loaded, so if you wanted the second rating to be after 10 kills, then then next 20 kills, this wouldn't work because there are values for 8 kills and 16 kills still being loaded from the original dictionary. You can only add to the existing keys or change the existing key values - you can't remove the old keys in an OXP. This, unfortunately, seems to kill off the dictionary approach as at least with the array approach from before you can define a whole new rating system - not just add to the existing one...

However going back to the array approach, there must be a way to sort the values properly, so that if they were not in order they could be put in order (without the game deciding this isn't allowed and reverting to the original hardcoded ratings system as it does now).

EDIT: I've also been trying to work on putting in an error if there is a negative value in the array. For example, this bit of code replaces the similar section in the ratings code:

Code: Select all

	if (!newRatingsInvalid)
	{
		for (i = 0; i < [ratingKills count]; ++i)
		{
			if ([ratingKills oo_unsignedIntAtIndex:i] < 0)
			{
				newRatingsInvalid = YES;
				OOLog(@"new.ratings.error", @"'oolite_rating_kills' array in descriptions.plist contains a negative value (lower than 0) - reverting to original ratings system.");
				break;
			}
			if (kills < [ratingKills oo_unsignedIntAtIndex:i])  
			{
				if ([ratingNames oo_stringAtIndex:i-1] != nil)
				{
					return [ratingNames oo_stringAtIndex:i-1];
				}
				else
				{
					newRatingsInvalid = YES;
					OOLog(@"new.ratings.error", @"'nil' value returned when trying to display rating status - reverting to original ratings system.");
					break;
				}
			}
		}
	}

However this doesn't seem to work, as even if I change the first value of the array from 0 to -5 it doesn't seem to pick this up. If I change if ([ratingKills oo_unsignedIntAtIndex:i] < 0 to if ([ratingKills oo_unsignedIntAtIndex:i] <= -1) It now always reverts to the hardcoded ratings system and puts an entry in the log that there is a negative value, even if there isn't!