Awaiting Info WorldMap: Serious Lapses in Coding Logic?

[Pieter Boelen]

The following was triggered by my investigations on this: Fixed - WorldMap Related Error.log Entry | Page 2 | PiratesAhoy!
And are probably related to this planned feature as well: Medium Priority - Worldmap Encounter Enhancement (V 1.32) Updated | PiratesAhoy!

I get the impression there is a definite ERROR with the GenerateMapEncounter function.
Refer to the following:

bool GenerateMapEncounter_Merchant(int iNearIslandNation, ref iEncounter)
{
  // find free slot in dynamic encounter table for map
   int iEncounterSlot = FindFreeMapEncounterSlot();
   if(iEncounterSlot == -1) return false;
   ManualReleaseMapEncounter(iEncounterSlot);

   ref rEncounter = &MapEncounters[iEncounterSlot];
   [...]
   iEncounter = iEncounterSlot;
   [...]
   return true;
}

Note that 'ref iEncounter' is not used as INPUT, because it gets SET in the function.
That means that it is probably meant to be an OUTPUT of the function that you can read with the "&" sign.

But this is where that function is called. Note that there is NO "&" sign to read the output on the marked line:

bool GenerateMapEncounter(int iMapEncounterType, string sIslandID, ref iEncounter1, ref iEncounter2)
{
   iEncounter1 = -1;
   iEncounter2 = -1;
[...]
         bReturn = GenerateMapEncounter_Merchant(iNearIslandNation, iEncounter1); // <------------------- NOTE THIS LINE -----------------------
[...]
   ref rEncounter1, rEncounter2;
   if(iEncounter1 != -1)
   {
     rEncounter1 = &MapEncounters[iEncounter1];
     rEncounter1.GroupName = ENCOUNTER_GROUP + iEncounter1;
   }
   if(iEncounter2 != -1)
   {
     rEncounter2 = &MapEncounters[iEncounter2];
     rEncounter2.GroupName = ENCOUNTER_GROUP + iEncounter2;
   }

   int i1 = iEncounter1;
   int i2 = iEncounter2;

   return true;
}

So also here, iEncounter1 is NOT used as input. Instead, it is set to '-1' at the beginning of the function and I see no spots where it is ever set to anything else.
I'd expect the marked line to be modified like this so that it can SET that variable:
bReturn = GenerateMapEncounter_Merchant(iNearIslandNation, &iEncounter1);
That would mean that 'iEncounter1' is set inside the GenerateMapEncounter_Merchant function and gets read as second output here.

Refer also to the final use here:

void wdmShipEncounter(float dltTime, float playerShipX, float playerShipZ, float playerShipAY)
{
   [...]
     int i1 = -1;
     int i2 = -1;
     [...]
       if(GenerateMapEncounter(WDM_ETYPE_MERCHANT, wdmGetCurrentIsland(), &i1, &i2) == false) return;
       wdmCreateMerchantShip(i1, "", rand(50) - 50);

Here 'i1' and 'i2' are read from the 'ref iEncounter1, ref iEncounter2' function inputs using the "&" sign.
Except we have already seen that those are ALWAYS '-1' as they never get set to anything else.

Note also that the variables have a different name in their use here: 'i1' and 'i2'.
This may appear to match with this section in the earlier function:

   int i1 = iEncounter1;
   int i2 = iEncounter2;

Except that it doesn't, because those variables are defined at the end of the function and never used.
They also are NOT returned to the function above that reads variables with the same names, because the variable names between functions DO NOT NEED TO MATCH.
'ref iEncounter1' from GenerateMapEncounter becomes 'int i1' in wdmShipEncounter because the "&" sign reads the (unused) input variable and NOT the variable of the same name.
See how these aren't even the same VARIABLE TYPE. A 'ref' suddenly gets used as an 'int' instead.

I have to admit I am no expert on programming theory, having learned virtually everything I know about it simply through doing it.
So @Levis, @jsv or @pedrwyth, please have a look at my above interpretation of this code and tell me if I'm making any lapses of logic here?

Assuming that I AM right here, then for sure this code is NOT serving its intended purpose.
Which makes me wonder how the worldmap encounters can even work as well as they do with the code in its current state?
I don't dare changing this code right now for fear of trying to fix a system that appears broken,
but suddenly finding out that the system as intended triggers all sorts of unintended side-effects.

 

[jsv]

Well... This code you've quoted looks old and I think it's more or less how the calls to GenerateMapEncounter are supposed to look:

void wdmShipEncounter(float dltTime, float playerShipX, float playerShipZ, float playerShipAY)
{
        //Идентификаторы энкоунтеров
        int i1 = -1;
        int i2 = -1;
        // [...] 
        if(GenerateMapEncounter(WDM_ETYPE_WARRING, wdmGetCurrentIsland(), &i1, &i2) == false) return;
        wdmCreateWarringShips(i1, i2);

The comment in Russian says i1 and i2 are "encounter identifiers", whatever that means.

The idea seems to be that wdmGetCurrentIsland returns some event-related integers (ship types in this case, but that would depend on an event) as its output parameters. So the caller provides GenerateMapEncounter with the references to some integer variables it owns, the callee sets those variables, and after that the caller uses the values for whatever they are good for.

Some of the code you've quoted earlier looks very strange, but whether it's badly broken or just strange-looking mostly depends on how argument passing is implemented in Storm engine. Hopefully, @Levis knows whether one can omit & when passing integer variable by reference in this language.

Otherwise we'll need to perform some experiments to figure that out.

 

[Pieter Boelen]

Well... This code you've quoted looks old and I think it's more or less how the calls to GenerateMapEncounter are supposed to look:

It is probably largely stock game code. But it IS the current code in Build 14 Beta 4.

As far as I understand the code, it wouldn't work as intended in its current form.
What is the difference between a 'reference' and an 'int' in the first place? I always figured they are two COMPLETELY DIFFERENT things.
Or is it some sort of pointer that refers to the 'int' inside the function?

But even then, it wouldn't work without READING that pointer with '&', would it?
And I'd say that if you WANT the values of the 'ints', why don't you feed the 'ints' themselves as function input/output variables,
rather than making a reference to the 'int' and then using it as if that reference IS the original 'int'.

 

[jsv]

It is probably largely stock game code. But it IS the current code in Build 14 Beta 4.

That "old" was used in a good sense of old, as in "hopefully written by someone with at least some understanding of how those things are supposed to work"

And I'd say that if you WANT the values of the 'ints', why don't you feed the 'ints' themselves as function input/output variables,
rather than making a reference to the 'int' and then using it as if that reference IS the original 'int'.

I know how those things work usually. I'm not familiar with the specifics of the Storm engine.

Here's your typical passing by value:

void foo(int arg)
{
    // doing something
    arg += 1;
}

int i = 10;
foo(i);

Here, the value of i, that is 10, is copied into arg when you call foo. Whatever foo then does to arg, it does to arg, not to i. When foo returns, the memory allocated for arg is deallocated and any changes to arg are lost.

And here is passing by reference (that's Storm-like pseudocode, I don't know whether it works this way here. That's more or less how it is in C++, only it would be "int&" instead of "ref" ):

void foo(ref arg)
{
    arg += 1;
}

int i = 10;
foo(&i);

Here, what is passed to foo is the address of the i variable (& meaning "take address of"). The way ref works, arg becomes an alias for i and whatever is done to arg is done to i.
And as i resides outside of the frame allocated for foo, changes to it are not lost when foo returns.


The value-passing works and looks more or less the same way in most languages, but there are countless variations in both syntax and semantics of reference-passing. Sometimes you're required to take the address of the variable you're passing explicitly, and sometimes it's taken implicitly whenever the corresponding argument is declared as reference type. In many languages, complex structures are always passed by reference, while small primitive objects (like int) are not... And so on.
How exactly it works in Storm, I don't know yet.

 

[Pieter Boelen]

Thanks for that!

And here is passing by reference (that's Storm-like pseudocode, I don't know whether it works this way here. That's more or less how it is in C++, only it would be "int&" instead of "ref" ):

void foo(ref arg)
{
    arg += 1;
}

int i = 10;
foo(&i);

Here, what is passed to foo is the address of the i variable (& meaning "take address of"). The way ref works, arg becomes an alias for i and whatever is done to arg is done to i.
And as i resides outside of the frame allocated for foo, changes to it are not lost when foo returns.

For some reason I've got in mind that it should work like this:

void foo(int arg) // <------- note that this in an 'int' here
{
    arg += 1;
}

int i = 10;
foo(&i);

So I understood that you simply use the variable type that you want.
And you can use the "&" sign to 'read' the value of any input variable.

So also:

void foo(string arg)
{
    arg = "test";
}

string temp;
foo(&temp);
LogIt("String variable = " + temp);

Maybe I'm misunderstanding it though...

 

[Pieter Boelen]

Looks like I'm NOT misunderstanding it. This totally works in PROGRAM\console.c:

void foo(string arg)
{
  arg = "test";
}

void ExecuteConsole()
{
   ref pchar = GetMainCharacter();
   ref ch;
   int i;
   int limit;
   
   string temp;
   foo(&temp);
   LogIt("String variable = " + temp);

Interestingly enough, this seems to ALSO work:

void ExecuteConsole()
{
   ref pchar = GetMainCharacter();
   ref ch;
   int limit;
   
   int i = 10;
   foo(&i);
   LogIt("Int variable = " + i);

And this works too:

void foo(ref arg)
{
  arg = "test";
}

void ExecuteConsole()
{
  ref pchar = GetMainCharacter();
  ref ch;
  int i;
  int limit;
   
  string temp;
  foo(&temp);
  LogIt("String variable = " + temp);

Unless I'm mistaken, this was actually faster in performance than if I define 'arg' as a string.

 

[Pieter Boelen]

But this is where that function is called. Note that there is NO "&" sign to read the output on the marked line:

bool GenerateMapEncounter(int iMapEncounterType, string sIslandID, ref iEncounter1, ref iEncounter2)
{
   iEncounter1 = -1;
   iEncounter2 = -1;
[...]
         bReturn = GenerateMapEncounter_Merchant(iNearIslandNation, iEncounter1); // <------------------- NOTE THIS LINE -----------------------
[...]
   ref rEncounter1, rEncounter2;
   if(iEncounter1 != -1)
   {
     rEncounter1 = &MapEncounters[iEncounter1];
     rEncounter1.GroupName = ENCOUNTER_GROUP + iEncounter1;
   }
   if(iEncounter2 != -1)
   {
     rEncounter2 = &MapEncounters[iEncounter2];
     rEncounter2.GroupName = ENCOUNTER_GROUP + iEncounter2;
   }

   int i1 = iEncounter1;
   int i2 = iEncounter2;

   return true;
}

So also here, iEncounter1 is NOT used as input. Instead, it is set to '-1' at the beginning of the function and I see no spots where it is ever set to anything else.
I'd expect the marked line to be modified like this so that it can SET that variable:
bReturn = GenerateMapEncounter_Merchant(iNearIslandNation, &iEncounter1);
That would mean that 'iEncounter1' is set inside the GenerateMapEncounter_Merchant function and gets read as second output here.

To return to the original subject, the section of code above looks to me like it needs a slight change:

bool GenerateMapEncounter(int iMapEncounterType, string sIslandID, ref iEncounter1, ref iEncounter2)
{
   iEncounter1 = -1;
   iEncounter2 = -1;
[...]
         bReturn = GenerateMapEncounter_Merchant(iNearIslandNation, &iEncounter1); // PB: Need to READ this variable with "&", otherwise it remains '-1'
[...]
   ref rEncounter1, rEncounter2;
   if(iEncounter1 != -1)
   {
     rEncounter1 = &MapEncounters[iEncounter1];
     rEncounter1.GroupName = ENCOUNTER_GROUP + iEncounter1;
   }
   if(iEncounter2 != -1)
   {
     rEncounter2 = &MapEncounters[iEncounter2];
     rEncounter2.GroupName = ENCOUNTER_GROUP + iEncounter2;
   }

//   int i1 = iEncounter1; // PB: This isn't used, because 'iEncounter1' is the variable that gets referenced, NOT this one
//   int i2 = iEncounter2; // PB: This isn't used, because 'iEncounter2' is the variable that gets referenced, NOT this one

   //Trace("Create encounter with slot " + iEncounter1 + ", Real = " + i1);
   //Trace("Create encounter with slot " + iEncounter2 + ", Real = " + i2);

   return true;
}

Possibly those last two lines were only meant to be used by those 'Trace' lines afterwards.

Still not sure what to make of those variable types. Refer to this code:

bool GenerateMapEncounter(int iMapEncounterType, string sIslandID, ref iEncounter1, ref iEncounter2)
{
   iEncounter1 = -1;

How would it know that 'iEncounter1' is meant to be an 'int' and not a 'ref'?
What IS a 'ref' of '-1'? Or is that just an empty reference?

 

[jsv]

Ok. So, from your console examples, 'ref' is a type (internally, probably a structure incapsulating a pointer). & makes that 'ref' of something. And operators like '=' change their behavior depending on the types of their operands (not a big surprise, given that + works differently for strings and numbers, for example).

If that's right, then it is NOT possible to omit & from a call. Try foo(temp) instead of foo(&temp). It's not working.
So we do have to look very closely at that GenerateMapEncounter code.
way hay an' up she rises

 

[jsv]

Ah, you're already looking at it

 

[Pieter Boelen]

Ah, you're already looking at it

Actually, I already stopped looking at it, since I figured some more feedback from you and/or @Levis might be in order.
And I don't want to risk actually breaking it; just want to make sure we know what, if anything, needs to be done.

 

[jsv]

Based on what we've seen in the console, I'd say that calls to GenerateMapEncounter_Merchant/War in Encounters_map.c indeed do not work as intended.

 

[jsv]

This language is a bit crazy.

Trying to do this:

int i = 42;
ref j = i;
j = 10;

gives you a run-time error.

But you're perfectly allowed to do this:

void foo (ref j) { j = 10; };
int i = 42;
foo(i);

This works (as in "not crashes"), but it doesn't change i, as one would expect.
So, yes, & seems to be mandatory when initializing ref arguments.

 

[Pieter Boelen]

This language is a bit crazy.

For certain!

Trying to do this:

int i = 42;
ref j = i;
j = 10;

gives you a run-time error.

Would I be correct in saying that this WOULD work?

int i = 42;
ref j = &i;
j = 10;

But you're perfectly allowed to do this:

void foo (ref j) { j = 10; };
int i = 42;
foo(i);

This works (as in "not crashes"), but it doesn't change i, as one would expect.
So, yes, & seems to be mandatory when initializing ref arguments.

In other words, this would do the trick:

[CODE]void foo (ref j) { j = 10; };
int i = 42;
foo(&i);

[/CODE]
Actually makes sense to me; then you can choose to either take the variable out of the function (with &) or only feed it into the function while keeping the original one intact.

 

[jsv]

Would I be correct in saying that this WOULD work?

int i = 42;
ref j = &i;
j = 10;

Yes.

In other words, this would do the trick:

[CODE]void foo (ref j) { j = 10; };
int i = 42;
foo(&i);

And yes.

Actually makes sense to me; then you can choose to either take the variable out of the function (with &) or only feed it into the function while keeping the original one intact.

There is nothing unusual about passing int where ref is expected behaving weirdly. It's just that it behaves weirdly in an inconsistent way.