Proposed Change/Addition to IsOfficer()

[Mere_Mortal]

The original IsOfficer() function serves only to query the player’s own officers. The problem with the original code is the use of GetMainCharacter(), which obviously targets the self-player and means it is useless for checking companions. Original...

bool IsOfficer(ref _refCharacter)
{
   if(CheckAttribute(_refCharacter, "index"))
   {
     int findIdx = sti(_refCharacter.index);
     if(findIdx >= 0)
     {
       for(int i=0; i<4; i++)
       {
         if(GetOfficersIndex(GetMainCharacter(),i) == findIdx)
           return true;
       }
     }
   }
   return false;
}

This is my own function which I have applied in a single case...

bool IsAnOfficer(ref _refShip, ref _refChar)
{
   if (CheckAttribute(_refChar,"index")) {
     int findIdx = sti(_refChar.index)
     if (findIdx > 0) {
       for (int i=0; i<OFFICER_MAX; i++) {
         if (GetOfficersIndex(_refShip,i) == findIdx) {
           return i // this effectively returns the officer's slot number
         }
       }
     }
   }
   return false
}

Instead of simply referencing the player, often MainChar or myCh, this permits the reference ship and the officer. As such, if enemyCh is the first input then that is where it will look. I use refShip1 and refShip2 in this respect because it’s easier to read it that way.

As to what is done with the original function, I guess there are two choices: remove it completely because it is obsolete in my view, or leave it in place because a lot of code will be referencing it and it’d be quite a pain to go through it all. Then there’s the case for including this function.

 

[Pieter Boelen]

If NPC ships are to have extra officers, then indeed that function might prove useful.

You could have the old function relay to your new one.
Then you don't need to change all function calls, but there won't be double code.

 

[Mere_Mortal]

I was actually trying to relay it funnily enough, but I was doing it wrong and it’s a pain to work with the global files because more often than not the game needs rebooting if there is even a minor discrepancy (but at least it refuses to load afterwards, so errors are generally obvious very quickly).

While we’re on this subject... what exactly is the point in using a character’s ID to make a reference to it, and then using that reference to find its ID? It seems a bit bonkers really.

Here’s an example...

int indChar = GetPassenger(myCh,numScroll)
ref refChar = GetCharacter(indChar)

Now if I want to see if this fellow is one of my officers, I can do that with IsOfficer(). I have to use refChar to this effect, but what does the function go and do? It gets the ID from the reference.
So why not just pass the ID in the first place?

bool IsOfficer(ref _refShip, int _indChar)
{
   for (int i=0; i<OFFICER_MAX; i++) {
     if (GetOfficersIndex(_refShip,i) == _indChar) {
     return true
     }
   }
   return false
}

I’m not sure if that’s correct, I wrote it here... but if it is then it just cut the function down by half.

In fact, GetOfficersIndex() probably does exactly the same thing so again we could most likely just pass the ship’s index as well to save some on the processing. In my view, this is terrible coding and if it’s occurring all over the place then there must surely be a point at which it will impact upon performance, particularly during combat.

 

[Pieter Boelen]

I think when you say "ID", you actually mean "index", right?

Nice thing about passing references around is that you can check all attributes on that character reference.
If you pass the index, you'd need to first change it to a reference before doing that.

I'm also inclined to not change stuff too much if it isn't broken.
There is so much that IS still broken or at least decidedly not good enough.
Large and ill-conceived changes have been rather a big pain for me over the past year or so and I'd rather not have that happen again....

 

[Mere_Mortal]

Yeah, I guess “ID” is a different attribute altogether. Is that their base name? Anyway, if the main purpose is to reference a character as a whole then that’s great, but for things like IsOfficer(), well that one seemingly has no reason at all to check any attributes. I get what you’re saying though, a massive overhaul of code could have two problems; one is that there’d potentially be lots and lots of errors, and even if it’s flawless then it would take a while for other people to get used to the changes, probably causing errors (or tantrums)!

Mind you, they often say “don’t fix what isn’t broken”... but what about bad habits?

 

[Pieter Boelen]

There is rather a lot of sloppy coding in the game, I'm afraid.
Some stems from the Unmodded version and after that of course a whole lot of different people worked on it.

If you want to sort that all out, you'll probably be going for a while....

 

[Mere_Mortal]

Haha... I don’t plan on going through it all, I’d rather just play the game.

 

[Pieter Boelen]

Haha... I don’t plan on going through it all, I’d rather just play the game.

Good thinking!

 

[Mere_Mortal]

Anyway, I have an idea... if it’ll work.

If a function expects two arguments, is it possible to still pass only one? In other words, instead of removing or relaying, the function would accept either scenario. So in the case of IsOfficer(), it will do one of two things:

1 argument: assume it is the character entity to be checked, defaulting to the main player (current behaviour)
2 arguments: take the first as above, but instead use the second as the base player to check it against

This I expect would be compatible with existing calls to the function.

 

[Pieter Boelen]

I don't think the PotC code allows for that.

What's the problem with relaying though?
I've done that quite recently on something else and that worked fine.

 

[Mere_Mortal]

The problem is exactly what I described: the original function expects one variable but if we are relaying it then it will be expecting two at some point. In other words, passing one variable would cease to work. At least I think I understand this correctly?

 

[Pieter Boelen]

You just use the original function with one Input variable to point top your new two variable one.
The extra variable is always the player character when called from the original version.

 

[Mere_Mortal]

That makes perfect sense!

By the way, if I do this...

string variable = "1"

Function(variable)

void Function(ref variable) {
   if (variable == true) {
     trace("Success")
   }
}

It will quite happily return as true, or false if the string is anything else.

If I swap the string and reference over then the interface locks up.

 

[Pieter Boelen]

Should they not just both be strings?

Except checking if a string is true/false also doesn't make sense.
Technically in that example it should all be 'bool'.

 

[Mere_Mortal]

Yeah of course, but my point is that the function didn’t care because it received a “1” anyway, even thought it expected a reference.

 

[Pieter Boelen]

If I understand correctly, it was a reference to a string and therefore effectively still a string.
Personally I would avoid using it like that, because it is SUPER confusing.
And not just to me, but I think also for the original game developers.

See this example:
Awaiting Info - WorldMap: Serious Lapses in Coding Logic? | PiratesAhoy!
I'm pretty sure there is some stuff in there that has been wrong since the original game.
It doesn't technically do any harm, but it probably doesn't function how it was meant to either.

 

[Mere_Mortal]

Personally I would avoid using it like that, because it is SUPER confusing.

Oh don’t get me wrong, I wouldn’t be using functions in that way... that is messy and wrong,
unless it’s some kind of bizarro improvisation to get around engine limitations.

 

[Pieter Boelen]

Oh don’t get me wrong, I wouldn’t be using functions in that way... that is messy and wrong,
unless it’s some kind of bizarro improvisation to get around engine limitations.

As far as I can tell, it is just weird coding and serves no specific purpose.

 

[Mere_Mortal]

I just realised something quite fascinating. When a query is done to see if somebody is an officer and he is, then the function returns as true. Well, I changed the “true” to an “i” and as a result it returned his officer slot number. So actually, this function is really useful because so long as the value is greater than zero then it returns as true, and we have his number! The catch is, and there always is one, that if the character happens to be the captain then it will come back false; I think that’s fine because he’s technically not an officer anyway.

 

[Pieter Boelen]

Where and how is the function used?
I know the code relies often on the fact that the player is Companion zero, for example.
Not sure if something similar applies to officers though.

Why do you want to know the exact officer number?