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?
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! 
