`feel()` sense does not seem to account for warrior movement during turn

[malob]

Environment

System:

• OS: macOS High Sierra 10.13.4
• CPU: x64 Intel(R) Core(TM) i7-7567U CPU @ 3.50GHz
• Memory: 1.40 GB / 16.00 GB
• Shell: 2.7.1 - /usr/local/bin/fish

  Binaries:

• Node: 8.9.2 - /usr/local/bin/node
• npm: 6.0.0 - /usr/local/bin/npm

Steps to reproduce

Run the following code on level 4 in the beginner tower:

class Player {
  constructor() {
    this.emptyAhead = true;
  }
  playTurn(w) {
    if (this.emptyAhead) { w.walk(); }
    else { w.walk('backward'); }
    this.emptyAhead = w.feel().isEmpty();
  }
}

Expected Behavior

First turn:

• this.emptyAhead is initialized as true so w.walk() is called at the start of the first turn.
• Now the Thick Sludge is immediately ahead of the warrior so w.feel().isEmpty() should return false which would set this.emptyAhead to false.

Second turn:

• At the start of turn 2 this.emptyAhead should be equal to false, and so w.walk('backward') should be called.

Welcome to WarriorJS
-------------------------------------- level 004 -------------------------------------
♥ 20
╔═══════╗
║@ Sa S>║
╚═══════╝
-------------------------------------- turn 001 --------------------------------------
♥ 20
╔═══════╗
║ @Sa S>║
╚═══════╝
> Malo walks forward
-------------------------------------- turn 002 --------------------------------------
♥ 20
╔═══════╗
║@ Sa S>║
╚═══════╝
> Malo walks backward
> Thick Sludge attacks forward and hits nothing

Actual Behavior

What actually seems to happen is that the feel() sense, if called after the warrior moves, does not account for the movement of the warrior. This behavior seems counterintuitive to me.

Welcome to WarriorJS
-------------------------------------- level 004 -------------------------------------
♥ 20
╔═══════╗
║@ Sa S>║
╚═══════╝
-------------------------------------- turn 001 --------------------------------------
♥ 20
╔═══════╗
║ @Sa S>║
╚═══════╝
> Malo walks forward
-------------------------------------- turn 002 --------------------------------------
♥ 17
╔═══════╗
║ @Sa S>║
╚═══════╝
> Malo walks forward
> Malo bumps into Thick Sludge
> Thick Sludge attacks forward and hits Malo
> Malo takes 3 damage, 17 health power left

[RascalTwo]

The problem is that senses are executed instantly when called, while actions are stored to be performed at the end of the turn.

https://github.com/olistic/warriorjs/blob/594712f0024ed5ae690b5a0aa96cd62d1464b8b4/packages/warriorjs-core/src/Level.js#L47-L48

You see, prepareTurn() eventually runs the player-defined playTurn(), which executes everything on the Turn object. All the senses get executed instantly, while the action is stored in the action variable of the Turn.

Then, in performTurn(), the turn.action is executed.

---

Actions are saved for later:

https://github.com/olistic/warriorjs/blob/23f60bf2a92689b9f73b1a3a796fc4a6e11e56fe/packages/warriorjs-core/src/Turn.js#L33-L43

while senses:

https://github.com/olistic/warriorjs/blob/23f60bf2a92689b9f73b1a3a796fc4a6e11e56fe/packages/warriorjs-core/src/Turn.js#L54-L59

are instantly executed.

---

Still thinking of a solution, someone else can suggest something at this point if they want.

---

The problem with delaying the execution of senses until the action is executed is the fact that the senses return values, and having the sense wait until an action is performed could be seen as a breakage of separation.

The problem with executing the action instantly is that while it works, it breaks the prepare and perform separation. Not to mention how it may damage other ordered events, like the Effects for example.

---

Given the structure and event flow, I don't even think senses are supposed to happen after actions in the first place, which may be why it doesn't work.

If this is true and senses aren't supposed to be executed after an action is performed, an error should be throw, something like Senses cannot be performed after actions.

[pigalot]

Would it not make sense to return the desired action from playTurn that way it would be clear that the action happens last. It also may be wise to seperate the actions so it is clear that they are actions.

class Player {
  constructor() {
    this.emptyAhead = true;
  }
  playTurn(w) {
    this.emptyAhead = w.feel().isEmpty();
    if (this.emptyAhead) { return w.actions.walk(); }
    else { return w.actions.walk('backward'); }
  }
}

[jpparent]

I agree with @pigalot. When I was playing, I always saw the action as the last thing to do in a turn (especially since you can only perform one action per turn). Thus I started to return them instead of having multiple else ifs.

[A1rPun]

I also agree with @pigalot. Maybe add some syntactic sugar like:

return warrior.walk // default forward
// or
return warrior.walk.backward

[olistic]

Thanks everyone here for the suggestions. Unfortunately, I don't have time to implement any major API changes right now.