Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mobx reaction unstable, cyclic function #528

Closed
hiaw opened this issue Sep 1, 2016 · 8 comments
Closed

mobx reaction unstable, cyclic function #528

hiaw opened this issue Sep 1, 2016 · 8 comments

Comments

@hiaw
Copy link

hiaw commented Sep 1, 2016

I'm trying to use MobX to create an Otello game. So I have reaction to look for tile changes and then update other tiles accordingly.

So in the code below, I ran bumpRandom() to change another tile to see the effect. But then this goes into cyclic function cause reaction is alway ran. How do I get it to stop observing in a reaction?

class OtelloBoard {
  @observable cells = [];

  constructor() {
    for (i = 0; i< SIZE*SIZE; i++) {
      this.cells.push(new Cell())
    }
    reaction(
      () => this.cells.map( cell => cell.status ),
      status => {
        this.bumpRandom()
        console.log('Status has changed' + status)
      }
    )
  }

  @action bumpRandom() {
    this.cells[45].bump()
  }
}

I tried untracked(() => this.bumpRandom()) but that doesn't work either.

@mweststrate
Copy link
Member

Can you explain what you want to achieve here? What needs to happen automatically here? To me this code reads as 'play the entire game', which mobx tries to do but then it sniffs an endless loop.

Should only one bump happen? Or only a bump when there is a specific status?

@hiaw
Copy link
Author

hiaw commented Sep 1, 2016

So when a tile is bumped, the reaction is called, it will then update another tile (bump). But this update should not trigger another reaction until it's finished. Maybe I should wrap them up in transactions?

@mweststrate
Copy link
Member

what is 'is finished'? you reaction is now configured to react to any change of status inside one of the cells

@hiaw
Copy link
Author

hiaw commented Sep 1, 2016

So I'm wanting to update other cells within the code block but I don't want that to cause a reaction. The reaction should only be triggered when user touches the cell.

reaction(
  () => this.cells.map( cell => cell.status ),
  status => {
    this.cells[45].status += 1          <-- This should not cause another reaction
  }
)

Is there a better way of doing what I want?

When user touches a cell, the code will react to it and change other cells.

@mweststrate
Copy link
Member

yep that shouldn't be a reaction in the first place, it sounds it sound triggered by a state change, but by a user event, in otherwords, an action. The idea of a reaction is that you can tell by just looking at the state what reactions should happen, which doesn't seem to be the case in your example, the reaction doesn't depend on the actual value of the status, by whom triggered it

@hiaw
Copy link
Author

hiaw commented Sep 1, 2016

Actually, I probably did not phrase it right.

When a cell value changes, the code will react to it and change other cells without triggering another reaction during those changes.

I'm relying on MobX to observe for state change. ie. when user touches the cell, the cell.status changes.

Maybe I shouldn't use mobx in this case?

@andykog
Copy link
Member

andykog commented Sep 1, 2016

@hiaw, you have a reaction, that depends on every cell, and when it fire it randomly changes cell, so will fire again untill it reaches stable state, which can't be achieved. transaction may help, but whats the point of using reaction here? You can simply call bumpRandom inside click handler, or you can make something like this, if you just want to use reaction somewhere:

class OtelloBoard {
  @observable cells = [];
  @observable move = 0;

  constructor() {
    for (i = 0; i< SIZE*SIZE; i++) {
      this.cells.push(new Cell())
    }
    reaction(
      () => this.move,
      move => {
        if (move % 2) {
          // computer move
          this.makeMove(Math.round(SIZE*SIZE * Math.random()))
        } else {
           // do nothing
        }
      }
    )
  }

  @action makeMove(cellNum) {
     this.cells[cellNum].status = !this.cells[cellNum].status
     this.move++;
  }
}

@hiaw
Copy link
Author

hiaw commented Sep 1, 2016

Yeah. I think you're right on using the click handler. Thank you very much for your help.

@hiaw hiaw closed this as completed Sep 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants