Need help with finding my mistake

[white]

So after a forced break due to a lack of time I'm back at coding. But I can't figure out why this code always chooses the first recruited ship, even when it's not close to the unit trying to enter a ship. The passengerCount of this ship can also go up to more than 3, which should be the max, when a fourth unit tries to enter any ship (and ends up in the first one). I hope someone who knows WML better than me can look at my code and point out my mistake(s) and maybe even provide a solution.

I should mention, that the variable shipNumber stores the total number of ships recruited in the game, savedX and savedY are saving the position of the last moving unit, so the option to enter a ship only shows up, when a unit actively moves next to a ship (which is intended). Also each ship gets a passengerCount of 0 on recruitment.

Code: Select all

[event]
  name=prestart
  [set_menu_item]
    id=entering
    description= _ "Enter ship"
    use_hotkeys=yes
    [default_hotkey]
      key=o
      alt=no
      shift=no
      ctrl=no
    [/default_hotkey]
    [show_if]
      [have_unit]
        x,y=$savedX,$savedY
        [filter_adjacent]
          type=Boat,Galleon,Pirate Galleon,Transport Galleon
          is_enemy=no
          [filter_wml]
            [variables]
              passengerCount=0
            [/variables]
          [/filter_wml]
          [or]
            [filter_wml]
              [variables]
                passengerCount=1
              [/variables]
            [/filter_wml]
          [/or]
          [or]
            [filter_wml]
              [variables]
                passengerCount=2
              [/variables]
            [/filter_wml]
          [/or]
        [/filter_adjacent]
      [/have_unit]
    [/show_if]
    [command]
      [store_unit]
        [filter]
          id=$unit.id
        [/filter]
        variable=enteringUnit
      [/store_unit]
      
      [store_unit]
        [filter]
          type=Boat,Galleon,Pirate Galleon,Transport Galleon
          side=$side_number
        [/filter]
        variable=ships
      [/store_unit]
      
      [set_variable]
        name=i
        value=0
      [/set_variable]
      
      [while]
        [variable]
          name=i
          less_than=$shipNumber
        [/variable]
        [do]
          [if]
            [have_unit]
              type=Boat,Galleon,Pirate Galleon,Transport Galleon
              side=$side_number
              [filter_wml]
                [variables]
                  passengerCount=0
                [/variables]
              [/filter_wml]
              [or]
                [filter_wml]
                  [variables]
                    passengerCount=1
                  [/variables]
                [/filter_wml]
              [/or]
              [or]
                [filter_wml]
                  [variables]
                    passengerCount=2
                  [/variables]
                [/filter_wml]
              [/or]
              [filter_adjacent]
                id=$enteringUnit.id
              [/filter_adjacent]
            [/have_unit]
            [then]
              [set_variable]
                name=newCount
                value=$ships[$i].variables.passengerCount
              [/set_variable]
              [set_variable]
                name=newCount
                add=1
              [/set_variable]
              [set_variable]
                name=oldCount
                value=$ships[$i].variables.passengerCount
              [/set_variable]
              [set_variable]
                name=ships[$i].variables.passengerCount
                value=$newCount
              [/set_variable]
              [store_unit]
                [filter]
                  id=$enteringUnit.id
                [/filter]
                variable=ships[$i].variables.passenger[$oldCount]
                kill=yes
              [/store_unit]
              
              [clear_variable]
                name=oldCount
              [/clear_variable]
              [clear_variable]
                name=newCount
              [/clear_variable]
              [set_variable]
                name=i
                value=50999
              [/set_variable]
            [/then]
          [/if]
          [set_variable]
            name=i
            add=1
          [/set_variable]
        [/do]
      [/while]
      
      [unstore_unit]
        variable=ships
      [/unstore_unit]
      
      [clear_variable]
        name=ships
      [/clear_variable]
      
      [clear_variable]
        name=enteringUnit
      [/clear_variable]
      
      [clear_variable]
        name=i
      [/clear_variable]
    [/command]
  [/set_menu_item]
[/event]

[Samonella]

Code: Select all

      [while]
        [variable]
          name=i
          less_than=$shipNumber
        [/variable]
        [do]
          [if]
            [have_unit]
              type=Boat,Galleon,Pirate Galleon,Transport Galleon
              side=$side_number
              [filter_wml]
                [variables]
                  passengerCount=0
                [/variables]
              [/filter_wml]
              [or]
                [filter_wml]
                  [variables]
                    passengerCount=1
                  [/variables]
                [/filter_wml]
              [/or]
              [or]
                [filter_wml]
                  [variables]
                    passengerCount=2
                  [/variables]
                [/filter_wml]
              [/or]
              [filter_adjacent]
                id=$enteringUnit.id
              [/filter_adjacent]
            [/have_unit]

Since the condition for this [if] has nothing to do with $i, [then] will happen on the first iteration through the loop. I'd add something like x,y=$ships[$i|].x|,$ships[$y|].y|

Also, imo a lot of this code would be easier to read/debug if you used {VARIABLE} and {VARIABLE_OP}. {CLEAR_VARIABLE} wouldn't hurt either.

Oh, and i just spotted this: [unstore_unit] can't unstore the entire array, only the first unit on top. You'll have to unstore each unit yourself. Which raises the question, why are you storing more than one ship in the first place? Why not just store the one that's adjacent to the unit?

Code: Select all

              [set_variable]
                name=i
                value=50999
              [/set_variable]

Now there's a clever way to break a loop! I can't believe everyone got by without [break] until 1.13.

[white]

Since the condition for this [if] has nothing to do with $i, [then] will happen on the first iteration through the loop. I'd add something like x,y=$ships[$i|].x|,$ships[$y|].y|

That sounds like a solution. Need to test it soon One question though: why are those pipes needed in that case?

Also, imo a lot of this code would be easier to read/debug if you used {VARIABLE} and {VARIABLE_OP}. {CLEAR_VARIABLE} wouldn't hurt either.

Well probably. I'm still new to WML, so I didn't understand those thingies in {} yet. That's why I didn't use any of them.

Oh, and i just spotted this: [unstore_unit] can't unstore the entire array, only the first unit on top. You'll have to unstore each unit yourself. Which raises the question, why are you storing more than one ship in the first place? Why not just store the one that's adjacent to the unit?

oh stupid me. I don't know why I am saving all ships... probably just an error in my thoughts lol

Now there's a clever way to break a loop! I can't believe everyone got by without [break] until 1.13.

thanks ^^

[Ravana]

In this case pipes are not needed, ] and , mark end of variable name already.

[Samonella]

Since the condition for this [if] has nothing to do with $i, [then] will happen on the first iteration through the loop. I'd add something like x,y=$ships[$i|].x|,$ships[$y|].y|

That sounds like a solution. Need to test it soon One question though: why are those pipes needed in that case?

In this case, those aren't pipelines, just termination characters. They just make it clear where the variable name ends and other stuff begins. I'm pretty sure in this case they aren't necessary, the compiler knows that ] and , also mean the variable name ended, but I do enough weird variable substitution stuff that I always include them out of habit. Details are here.

Also, imo a lot of this code would be easier to read/debug if you used {VARIABLE} and {VARIABLE_OP}. {CLEAR_VARIABLE} wouldn't hurt either.

Well probably. I'm still new to WML, so I didn't understand those thingies in {} yet. That's why I didn't use any of them.

They're called macros, and they're literally just shortcuts. They just substitute in some wml so you don't have to write/read it all yourself. You can always see exactly what they substitute in by looking at the right file in data/core/macros/
In this case, {VARIABLE} is in data/core/macros/utils.cfg, in which you can find this:

Code: Select all

#define VARIABLE VAR VALUE
    # Macro to initialize a variable.  Strictly a syntatic shortcut.
    [set_variable]
        name={VAR}
        value={VALUE}
    [/set_variable]
#enddef

which means that wherever you type {VARIABLE i 50999} it is literally replaced with

Code: Select all

    [set_variable]
        name=i
        value=50999
    [/set_variable]

before the game actually compiles your code. You can also define your own macros wherever/whenever you want, using the same syntax that data/core/macros/utils.cfg does.

[white]

Ok, so after changing the

Code: Select all

$ships[$y|].y|

part with

Code: Select all

$ships[$i].y

it didn't save the entering unit in the first ship anymore, but even after adding

Code: Select all

  [set_variable]
        name=i
        value=0
      [/set_variable]
      
      [while]
        [variable]
          name=i
          less_than=$shipNumber
        [/variable]
        [do]
          [unstore_unit]
            variable=ships
          [/unstore_unit]
          [set_variable]
            name=i
            add=1
          [/set_variable]
        [/do]
      [/while]

it doesn't save the unit in the second ship. Any clues why it doesn't?

ps: also thanks for the short explanation of macros. Need to get used to them asap

[Samonella]

Code: Select all

          [unstore_unit]
            variable=ships
          [/unstore_unit]

Change this to

Code: Select all

    [unstore_unit]
        variable=ships[$i]
    [/unstore_unit]

and I think that'll do it. Although if it was me, I'd remove both loops and only store the ship that's adjacent to the unit in the first place.

[white]

Change this to

Code: Select all
[unstore_unit]
variable=ships[$i]
[/unstore_unit]

and I think that'll do it.

sometimes I'm stupid lol

[white]

ok, finally I got it working. I just wonder why my other attempt failed... so maybe someone is able to explain it to me.
My working code fragment:

Code: Select all

      [while]
        [variable]
          name=i
          less_than=$shipNumber
        [/variable]
        [do]
          [if]
            [have_unit]
              type=Boat,Galleon,Pirate Galleon,Transport Galleon
              side=$side_number
              x,y=$ships[$i].x,$ships[$i].y
              [filter_wml]
                [variables]
                  passengerCount=0
                [/variables]
              [/filter_wml]
              [filter_adjacent]
                id=$enteringUnit.id
              [/filter_adjacent]
            [/have_unit]
            [or]
              [have_unit]
                type=Boat,Galleon,Pirate Galleon,Transport Galleon
                side=$side_number
                x,y=$ships[$i].x,$ships[$i].y
                [filter_wml]
                  [variables]
                    passengerCount=1
                  [/variables]
                [/filter_wml]
                [filter_adjacent]
                  id=$enteringUnit.id
                [/filter_adjacent]
              [/have_unit]
            [/or]
            [or]
              [have_unit]
                type=Boat,Galleon,Pirate Galleon,Transport Galleon
                side=$side_number
                x,y=$ships[$i].x,$ships[$i].y
                [filter_wml]
                  [variables]
                    passengerCount=2
                  [/variables]
                [/filter_wml]
                [filter_adjacent]
                  id=$enteringUnit.id
                [/filter_adjacent]
              [/have_unit]
            [/or]
            [then]
              [set_variable]
                name=newCount
                value=$ships[$i].variables.passengerCount
              [/set_variable]
              [set_variable]
                name=newCount
                add=1
              [/set_variable]
              [set_variable]
                name=oldCount
                value=$ships[$i].variables.passengerCount
              [/set_variable]
              [set_variable]
                name=ships[$i].variables.passengerCount
                value=$newCount
              [/set_variable]
              [store_unit]
                [filter]
                  id=$enteringUnit.id
                [/filter]
                variable=ships[$i].variables.passenger[$oldCount]
                kill=yes
              [/store_unit]
              
              [clear_variable]
                name=savedX
              [/clear_variable]
              [clear_variable]
                name=savedY
              [/clear_variable]
              [clear_variable]
                name=oldCount
              [/clear_variable]
              [clear_variable]
                name=newCount
              [/clear_variable]
              [set_variable]
                name=i
                value=50999
              [/set_variable]
            [/then]
          [/if]
          [set_variable]
            name=i
            add=1
          [/set_variable]
        [/do]
      [/while]

and the failing attempt (first unit entered the correct boat, but every unit trying to enter the same boat afterwards just ended up in the first boat recruited...)

Code: Select all

      [while]
        [variable]
          name=i
          less_than=$shipNumber
        [/variable]
        [do]
          [if]
            [have_unit]
              type=Boat,Galleon,Pirate Galleon,Transport Galleon
              side=$side_number
              x,y=$ships[$i].x,$ships[$i].y
              [filter_wml]
                [variables]
                  passengerCount=0
                [/variables]
              [/filter_wml]
              [or]
                [filter_wml]
                  [variables]
                    passengerCount=1
                  [/variables]
                [/filter_wml]
              [/or]
              [or]
                [filter_wml]
                  [variables]
                    passengerCount=2
                  [/variables]
                [/filter_wml]
              [/or]
              [filter_adjacent]
                id=$enteringUnit.id
              [/filter_adjacent]
            [/have_unit]
            [then]
              [set_variable]
                name=newCount
                value=$ships[$i].variables.passengerCount
              [/set_variable]
              [set_variable]
                name=newCount
                add=1
              [/set_variable]
              [set_variable]
                name=oldCount
                value=$ships[$i].variables.passengerCount
              [/set_variable]
              [set_variable]
                name=ships[$i].variables.passengerCount
                value=$newCount
              [/set_variable]
              [store_unit]
                [filter]
                  id=$enteringUnit.id
                [/filter]
                variable=ships[$i].variables.passenger[$oldCount]
                kill=yes
              [/store_unit]
              
              [clear_variable]
                name=savedX
              [/clear_variable]
              [clear_variable]
                name=savedY
              [/clear_variable]
              [clear_variable]
                name=oldCount
              [/clear_variable]
              [clear_variable]
                name=newCount
              [/clear_variable]
              [set_variable]
                name=i
                value=50999
              [/set_variable]
            [/then]
          [/if]
          [set_variable]
            name=i
            add=1
          [/set_variable]
        [/do]
      [/while]

so why did that if-condition not work while the other does?

[Samonella]

Ah, i didn't notice that before. It's a common mistake, one I've made a few times. So the filter you meant to have originally was this:

Code: Select all

            [have_unit]
              type=Boat,Galleon,Pirate Galleon,Transport Galleon
              side=$side_number
              x,y=$ships[$i].x,$ships[$i].y
              [filter_adjacent]
                id=$enteringUnit.id
              [/filter_adjacent]
              [and]
                [filter_wml]
                  [variables]
                    passengerCount=0
                  [/variables]
                [/filter_wml]
                [or]
                  [filter_wml]
                    [variables]
                      passengerCount=1
                    [/variables]
                  [/filter_wml]
                [/or]
                [or]
                  [filter_wml]
                    [variables]
                      passengerCount=2
                    [/variables]
                  [/filter_wml]
                [/or]
              [/and]
            [/have_unit]

Notice that the [and] separates all the terms that should be affected by the [or], so the terms above the [and] must be true no matter what.

The old, faulty filter was passed whenever passengerCount=1 or 2, since it was evaluated like this:

the unit must have the right side, type, adjacent unit, x,y, and passengerCount=0
[or]
passengerCount=1
[or]
passengerCount=2.

Your new filter works because it was basically saying:

the unit must have the right side, type, adjacent unit, x,y, and passengerCount=0
[or]
the unit must have the right side, type, adjacent unit, x,y, and passengerCount=1
[or]
the unit must have the right side, type, adjacent unit, x,y, and passengerCount=2.

For comparison, the one I suggested (which is what your original one was meant to do) does this (unless there's a typo):

the unit must have the right side, type, adjacent unit, x,y,
[and]
passengerCount=1 or passengerCount=2 or passengerCount=3

[white]

ah. Explained like this it seems so obvious that I'm wondering myself now how I could miss that lol well thanks for your explanation / help

also I encountered a new problem by now: how can I create a unit that is for example poisoned when created? And how do I copy the statuses of one unit to a new one?
(I'm not sure if I should open a new topic for that question, so feel free to tell me that I should do that ^^)

[Samonella]

also I encountered a new problem by now: how can I create a unit that is for example poisoned when created?

After glancing through the wiki for SingleUnitWML, looks like you can just put [status]poisoned=yes[/status] in the [unit] tag.

And how do I copy the statuses of one unit to a new one?

Try looking at the wml of a stored unit by using :debug and :inspect. You can see just what gets stored in a unit's variable. From that, you should be able to store the two units and copy one's statuses into the other.

(I'm not sure if I should open a new topic for that question, so feel free to tell me that I should do that ^^)

I think the recommended thing to do is change the topic title (by editing the first post's title) to something like "White's wml questions, currently <solved/unsolved>"

[white]

well I figured it out now. Thanks for all the help!