goban icon indicating copy to clipboard operation
goban copied to clipboard

implementation of methods to examination of the game

Open korintje opened this issue 4 years ago • 6 comments

What has been added

  • goban::rules::game::Game::move_num
  • goban::rules::game::Game::moves_history // Record of stone moves (not record goban states or other parameters)
  • goban::rules::game::Game::initialize()
  • goban::rules::game::Game::update_moves_history()
  • goban::rules::game::Game::load_by_movenum
  • goban::rules::game::Game::move_backward()
  • goban::rules::game::Game::move_forward()

Purpose

I think it is beneficial for users if goban::pieces::goban::Goban is updatable along the history, because go-players often examine their games by repeatably moving backward, forward, or making a new branch from a certain point. Newly-added methods allow library users to freely move in the game history and start new game branch, Both move_backward() and move_forward() update goban::pieces::goban::Goban, but do not update moves_history. When users use goban::rules::game::Game::play(play), the moves_history will be updated. Therefore, the users can start newly branched game, like below:

use goban;
use rand::seq::IteratorRandom;

fn main () {

    let mut g = goban::rules::game::Game::builder()
    .size((19,19))
    .rule(goban::rules::Rule::Chinese)
    .build().unwrap();

    let mut i = 4;
    while !g.is_over() && i != 0 {
        g.play(
            g.legals()
                .choose(&mut rand::thread_rng())
                .map(|point| goban::rules::Move::Play(point.0,point.1))
                .unwrap());
        i -= 1;
        g.display_goban();
    }

    for _j in 0..5 {
        g.move_backward();
        g.display_goban();
    }
    
    for _k in 0..2 {
        g.move_forward();
        g.display_goban();
    }

    let mut i = 6;
    while !g.is_over() && i != 0 {
        g.play(
            g.legals()
                .choose(&mut rand::thread_rng())
                .map(|point| goban::rules::Move::Play(point.0,point.1))
                .unwrap());
        i -= 1;
        g.display_goban();
    }

}

Points to be discussed

  • Are these features much to this crate?
  • Mixture of goban::rules::game::Game::history and goban::rules::game::Game::moves_history may be not smart. However, (in my understanding) goban::rules::game::Game::history contains only goban states, not all game states (like prisoners or ko-point). Therefore, it is not suitable for the above-mentioned purpose.
  • Instead of recording moves_history, we have an option to record goban::rules::game::Game directly. Maybe the former is more memory-efficient, and the later is more CPU-efficient. Which do you think better?
  • I do not have good idea for the name of goban::rules::game::Game::__play__ ......
  • My Rust code may be still awkward.

korintje avatar Sep 01 '20 20:09 korintje

Hi, thanks for your work. So I think this feature is welcome to the crate, I always wanted to export to SGF.

I seriously think that we may add to many complexity in the Game struct. I think a new struct GameTree can be usefull. We keep Game as an push-only structure without the historic. and the GameTree can handle all the branches an all. move_backwards is climbing the tree, and move_forward is a problem because how we know which branch are we in. load_by_move_num can be awkward too.

struct GameTree {
     tree: Tree<Game>>,
     actual: Node
     hash_map : HashMap<Game,Node>
}

impl GameTree{
      fn new(initial_state:Game){
      }
      fn play(move: Move){} // creates a new branch if the game doesn't already exist.
      fn actual_game() -> &Game;
}

Sagebati avatar Sep 03 '20 11:09 Sagebati

Hi, thanks for your review! I totally agree with your opinion.

I have imagined that once new branch is started by game::play(), the former branch will be removed. Always only one history is stored (So, I should not have call it "branch"), and it can be climbed up, down, and restarted from any points. Anyway, I changed my mind that it might be too much ad hoc to this crate.

On the other hand, GameTree struct, which you showed me, sounds much sophisticated and applicable in general. It will be beneficial for examining a game or searching the best moves by AI. I agree with your opinion that goban crate should adopt it.

I found that you start a new project. It seems to be deeply related to this function. Therefore, let me close this Pull requests as is. I think I can contribute this project in another way, implementation of export_sgf.

Many thanks!

korintje avatar Sep 03 '20 12:09 korintje

My idea for GameTree was to imitate this behaviour (see image), to have a nice way to handle game analysis. Furthermost an sfg_export feature needs to manage trees because that how the format is, and is handled in the library. The GameTree only function would have been to iterate through analysis and add branches. I'm sorry if I misunderstood your view. image

Sagebati avatar Sep 03 '20 14:09 Sagebati

I got your idea. I'm sorry for misunderstanding what you want to say. (And, as you say, implementation of GameTree should be dealt with before sgf_export .)

So, could you give me an advice?

  • If you have not yet go on to this work, and you think I can help you, I will re-open this Pull Request and work with it. Now I understood your idea, so I can deal with it.
  • Instead, if you have already implemented a good deal of the GameTree, or if you think it should be implemented by the maintainer, I'd love to leave it to you.

korintje avatar Sep 03 '20 15:09 korintje

As you want ! for the moment I don't know how to properly implement this idea (a Tree or a Vec<Rc<Game>>...). If you have an idea we can discuss it. However if you want to implement the sgf_export for Game with the historic feature I'm fine with it. Maybe it will be re-implemented when we get a GameTree, but that doesn't matter. For the rest I prefer to wait the GameTree structure to correctly handle an analysis tree. Thanks for your time.

ps: I discovered some bugs in the lib so stay updated.

Sagebati avatar Sep 04 '20 14:09 Sagebati

Thank you for your reply. OK, I will work on that. After I have implemented it to the coming version, I will beg your check again.

Many thanks!

korintje avatar Sep 04 '20 14:09 korintje