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

implementation of methods to examination of the game #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

korintje
Copy link

@korintje korintje commented Sep 1, 2020

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.

@Sagebati
Copy link
Owner

Sagebati commented Sep 3, 2020

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;
}

@korintje
Copy link
Author

korintje commented Sep 3, 2020

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 korintje closed this Sep 3, 2020
@korintje korintje deleted the korintje-branch branch September 3, 2020 12:57
@Sagebati
Copy link
Owner

Sagebati commented Sep 3, 2020

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

@korintje
Copy link
Author

korintje commented Sep 3, 2020

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.

@Sagebati
Copy link
Owner

Sagebati commented Sep 4, 2020

As you want ! for the moment I don't know how to properly implement this idea (a Tree or a Vec<Rc>...). 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.

@korintje korintje restored the korintje-branch branch September 4, 2020 14:16
@korintje
Copy link
Author

korintje commented Sep 4, 2020

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 korintje reopened this Sep 4, 2020
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

Successfully merging this pull request may close these issues.

2 participants