patternMinor
Tic-Tac-Toe game using erlang gen_fsm
Viewed 0 times
erlangtoegen_fsmtictacgameusing
Problem
I would like to use erlang to create an online, web-based game. I thought it would make sense to start with something easy, so I chose tic-tac-toe (which seems like a popular choice here). I am implementing users, players and the rules engine as finite state machines using the OTP gen_fsm.
The first step I decided to take is to get all the FSM's in order. I have one for users (i.e., someone logged in to the web site), one for players (if we decide to allow users having more than 1 player), and one for a game. I would like to know if I have some obviously poor choices in my implementation of the finite state machines. Any other un-Erlang like choices should also be pointed out.
The User
It is now mostly a placeholder. I am planning on using an erlang web framework, so the Cowboy application will handle the creation of the User processes. The module is called
```
-module(r_user).
-behaviour(gen_server).
-export([start/1, start_link/1, init/1, handle_call/3, handle_cast/2, handle_info/2, terminate/2, code_change/3]).
-export([get_name/1, log_out/1]).
-export([test/0]).
-record(?MODULE, {
name = []
}).
new([Name]) ->
#?MODULE {
name = Name
}.
start(Name) ->
gen_server:start(?MODULE, [Name], []).
start_link(Name) ->
gen_server:start_link(?MODULE, [Name], []).
init([Name]) ->
process_flag(trap_exit, true),
{ok, new([Name])}.
handle_call(terminate, _From, State) ->
{stop, normal, ok, State};
handle_call(getname, _Pid, State) ->
{reply, State#?MODULE.name, State};
handle_call(Event, From, State) ->
io:format("Received unknown call ~p from ~p, state = ~p~n", [Event, From, State]),
{noreply, State}.
handle_cast(Request, State) ->
io:format("Received unknown cast ~p, state = ~p~n", [Request, State]),
{noreply, State}.
handle_info(Info, State) ->
io:format("Received unknown info ~p, state = ~
The first step I decided to take is to get all the FSM's in order. I have one for users (i.e., someone logged in to the web site), one for players (if we decide to allow users having more than 1 player), and one for a game. I would like to know if I have some obviously poor choices in my implementation of the finite state machines. Any other un-Erlang like choices should also be pointed out.
The User
It is now mostly a placeholder. I am planning on using an erlang web framework, so the Cowboy application will handle the creation of the User processes. The module is called
r_user (because I believe there already is a user.erl module that is part of the base erlang distribution.```
-module(r_user).
-behaviour(gen_server).
-export([start/1, start_link/1, init/1, handle_call/3, handle_cast/2, handle_info/2, terminate/2, code_change/3]).
-export([get_name/1, log_out/1]).
-export([test/0]).
-record(?MODULE, {
name = []
}).
new([Name]) ->
#?MODULE {
name = Name
}.
start(Name) ->
gen_server:start(?MODULE, [Name], []).
start_link(Name) ->
gen_server:start_link(?MODULE, [Name], []).
init([Name]) ->
process_flag(trap_exit, true),
{ok, new([Name])}.
handle_call(terminate, _From, State) ->
{stop, normal, ok, State};
handle_call(getname, _Pid, State) ->
{reply, State#?MODULE.name, State};
handle_call(Event, From, State) ->
io:format("Received unknown call ~p from ~p, state = ~p~n", [Event, From, State]),
{noreply, State}.
handle_cast(Request, State) ->
io:format("Received unknown cast ~p, state = ~p~n", [Request, State]),
{noreply, State}.
handle_info(Info, State) ->
io:format("Received unknown info ~p, state = ~
Solution
I read your code carefully, and I must say it is easy to read. I have some remarks to make though, sorted from the most to the least important (at least from my point of view :o).
The first one is about the definition of roles and startup phase. In my opinion you should clarify first how the system works: creation of a game, a user, how he connects etc. Your system works, but everything must be done manually, and you have 2 processes for the player which roles aren't very clear, and which store in their state useless information - I think you could work with the player only, storing the game and its name. In my opinion you should have one registered server whose job is to register the users, create games on demand etc. If I have time I will propose you something. You should use the OTP supervisor behavior to manage the processes (and application to startup the whole thing).
You have used
You use
You don't take advantage of the very powerful Erlang pattern matching. It is frequent to write more clauses of one function, each of them with limited code and cases. It is a good usage since it allows to write he code for different use cases without modifying the previous version, decode in the clause most of the variable you will need, and in general it gives very compact, but still readable, code.
You used a list to store the state of the game. For a small and fixed size variable I prefer to use a tuple, like this it is clear that the size will not change. The list could be a good choice if the players can choose the size of the board at the beginning.
Last there is a bug using the function
Here I propose you a new version of your code (I removed r_user). This version checks most of the erroneous cases and warns the player. I didn't enhance the startup, I'll do it if I find time. It is followed by an example of game performed on 3 different VMs connected in a cluster.
The player:
```
-module(player).
-behaviour(gen_fsm).
%%% Public API
%% Behavior functions
-export([start/1, start_link/1]).
%% gen_fsm callbacks
-export([
% Behaviour callbacks
init/1, handle_event/3, handle_sync_event/4, handle_info/3, terminate/3, code_change/4,
% State functions
waiting_for_game/2, in_game/2, in_game/3
]).
%% Utility functions
-export([
get_name/1, quit/1, game_quit/1, make_move/3
]).
%%% State Record
-record(?MODULE, {
user,
game = none
}).
%% Behavior functions
start(User) ->
gen_fsm:start(?MODULE, [User], []).
start_link(User) ->
gen_fsm:start_link(?MODULE, [User], []).
%% Behavior callbacks
init([User]) ->
{ok, waiting_for_game, new(User)}.
handle_event(stop, _StateName, Data) ->
{stop, normal, Data};
handle_event(Event, StateName, Data) ->
unexpected(Event, StateName, Data),
{next_state, StateName, Data}.
handle_sync_event(getname, _From, StateName, Data) ->
{reply, Data#?MODULE.user, StateName, Data};
handle_sync_event(Event, _From, StateName, Data) ->
unexpected(Event, StateName, Data),
{next_state, StateName, Data}.
handle_info(Info, StateName, Data) ->
unexpected(Info, StateName, Data),
{next_state, StateName, Data}.
code_change(_OldVsn, StateName, Data, _Extra) ->
{ok, StateName, Data}.
terminate(normal, StateName, State=#?MODULE{}) ->
io:format("Player terminated normally, state name = ~p, ~p~n", [StateName, State]);
terminate(Reason, StateName, State) ->
io:format("Player terminated abnormally, reason = ~p, state name = ~p, state = ~p~n", [Reason, StateName, State]).
%% State functions
waiting_for_game({gamejoined, Game}, Data) ->
io:format("Player ~s: joined game ~p/~p~n", [D
The first one is about the definition of roles and startup phase. In my opinion you should clarify first how the system works: creation of a game, a user, how he connects etc. Your system works, but everything must be done manually, and you have 2 processes for the player which roles aren't very clear, and which store in their state useless information - I think you could work with the player only, storing the game and its name. In my opinion you should have one registered server whose job is to register the users, create games on demand etc. If I have time I will propose you something. You should use the OTP supervisor behavior to manage the processes (and application to startup the whole thing).
You have used
process_flag(trap_exit, true) in all your processes. You should not do this. There are very few reason why a process die. The most common are your bugs, and trapping exit will not help you to debug or live with them. Then there are some environment issues (memory, disk, OS, other application...) and it will not help in this case either. Especially in your case because you do not analyze the potential error messages. This feature is generally reserved to supervisors. In your code you should add some defensive measure to prevent the bad input from external world (early crash, adding some test or try/catch + user interaction...)You use
case in a somewhat wrong way. I consider that 3 intricate cases is not a good idea in Erlang. And there is a bug in your code maybe due to that: you forgot that the result of the main case must return something like {next_state,StateName,State}. In 2 branches, it ends on io:format(...) that returns ok and causes a crash of the fsm.You don't take advantage of the very powerful Erlang pattern matching. It is frequent to write more clauses of one function, each of them with limited code and cases. It is a good usage since it allows to write he code for different use cases without modifying the previous version, decode in the clause most of the variable you will need, and in general it gives very compact, but still readable, code.
You used a list to store the state of the game. For a small and fixed size variable I prefer to use a tuple, like this it is clear that the size will not change. The list could be a good choice if the players can choose the size of the board at the beginning.
Last there is a bug using the function
random:uniform/1. This function uses a seed to generate pseudo random sequences. The seed is valid at process level (in fact the last value is stored in the process dictionary, which is a "side effect", but linked to the expected behavior). If you don't initialize it with something like random:seed(erlang:now()), the function will always return the same sequence. Here I propose you a new version of your code (I removed r_user). This version checks most of the erroneous cases and warns the player. I didn't enhance the startup, I'll do it if I find time. It is followed by an example of game performed on 3 different VMs connected in a cluster.
The player:
```
-module(player).
-behaviour(gen_fsm).
%%% Public API
%% Behavior functions
-export([start/1, start_link/1]).
%% gen_fsm callbacks
-export([
% Behaviour callbacks
init/1, handle_event/3, handle_sync_event/4, handle_info/3, terminate/3, code_change/4,
% State functions
waiting_for_game/2, in_game/2, in_game/3
]).
%% Utility functions
-export([
get_name/1, quit/1, game_quit/1, make_move/3
]).
%%% State Record
-record(?MODULE, {
user,
game = none
}).
%% Behavior functions
start(User) ->
gen_fsm:start(?MODULE, [User], []).
start_link(User) ->
gen_fsm:start_link(?MODULE, [User], []).
%% Behavior callbacks
init([User]) ->
{ok, waiting_for_game, new(User)}.
handle_event(stop, _StateName, Data) ->
{stop, normal, Data};
handle_event(Event, StateName, Data) ->
unexpected(Event, StateName, Data),
{next_state, StateName, Data}.
handle_sync_event(getname, _From, StateName, Data) ->
{reply, Data#?MODULE.user, StateName, Data};
handle_sync_event(Event, _From, StateName, Data) ->
unexpected(Event, StateName, Data),
{next_state, StateName, Data}.
handle_info(Info, StateName, Data) ->
unexpected(Info, StateName, Data),
{next_state, StateName, Data}.
code_change(_OldVsn, StateName, Data, _Extra) ->
{ok, StateName, Data}.
terminate(normal, StateName, State=#?MODULE{}) ->
io:format("Player terminated normally, state name = ~p, ~p~n", [StateName, State]);
terminate(Reason, StateName, State) ->
io:format("Player terminated abnormally, reason = ~p, state name = ~p, state = ~p~n", [Reason, StateName, State]).
%% State functions
waiting_for_game({gamejoined, Game}, Data) ->
io:format("Player ~s: joined game ~p/~p~n", [D
Code Snippets
-module(player).
-behaviour(gen_fsm).
%%% Public API
%% Behavior functions
-export([start/1, start_link/1]).
%% gen_fsm callbacks
-export([
% Behaviour callbacks
init/1, handle_event/3, handle_sync_event/4, handle_info/3, terminate/3, code_change/4,
% State functions
waiting_for_game/2, in_game/2, in_game/3
]).
%% Utility functions
-export([
get_name/1, quit/1, game_quit/1, make_move/3
]).
%%% State Record
-record(?MODULE, {
user,
game = none
}).
%% Behavior functions
start(User) ->
gen_fsm:start(?MODULE, [User], []).
start_link(User) ->
gen_fsm:start_link(?MODULE, [User], []).
%% Behavior callbacks
init([User]) ->
{ok, waiting_for_game, new(User)}.
handle_event(stop, _StateName, Data) ->
{stop, normal, Data};
handle_event(Event, StateName, Data) ->
unexpected(Event, StateName, Data),
{next_state, StateName, Data}.
handle_sync_event(getname, _From, StateName, Data) ->
{reply, Data#?MODULE.user, StateName, Data};
handle_sync_event(Event, _From, StateName, Data) ->
unexpected(Event, StateName, Data),
{next_state, StateName, Data}.
handle_info(Info, StateName, Data) ->
unexpected(Info, StateName, Data),
{next_state, StateName, Data}.
code_change(_OldVsn, StateName, Data, _Extra) ->
{ok, StateName, Data}.
terminate(normal, StateName, State=#?MODULE{}) ->
io:format("Player terminated normally, state name = ~p, ~p~n", [StateName, State]);
terminate(Reason, StateName, State) ->
io:format("Player terminated abnormally, reason = ~p, state name = ~p, state = ~p~n", [Reason, StateName, State]).
%% State functions
waiting_for_game({gamejoined, Game}, Data) ->
io:format("Player ~s: joined game ~p/~p~n", [Data#?MODULE.user, game:get_number(Game), Game]),
NewState = Data#?MODULE{game = Game},
{next_state, in_game, NewState};
waiting_for_game(Event, State) ->
io:format("waiting_for_game: received unknown event ~p, state = ~p~n", [Event, State]),
{next_state, waiting_for_game, State}.
in_game(gamequit, Data) ->
Game = Data#?MODULE.game,
io:format("Player ~s: quit game ~p~n", [Data#?MODULE.user, Game]),
{next_state, waiting_for_game, Data#?MODULE{game = none}};
in_game({gameover, Winner}, Data) ->
Game = Data#?MODULE.game,
Result = case Winner of
winner -> "you won";
looser -> "you loose";
par -> "there is no winner"
end,
io:format("~s: ~p Game over, ~s~n", [Data#?MODULE.user, Game, Result]),
{next_state, waiting_for_game, Data#?MODULE{game = none}};
in_game({yourmove, Game}, State) ->
print_play(State#?MODULE.user,Game),
{next_state, in_game, State};
in_game({makemove, Row, Column}, State) ->
Game = State#?MODULE.game,
io:format("in_game: setting row ~p, column ~p~n", [Row, Column]),
gen_fsm:send_event(Game, {playeraction, self(), {Row, Column}}),
{next_state, in_game, State};
in_game(gotmove, State) ->
io:format("movement accepted~n"),
{next_sta-module(game).
-behaviour(gen_fsm).
%%% Public API
%% Behavior functions
-export([start/1, start_link/1]).
%% gen_fsm callbacks
-export([
% Behaviour callbacks
init/1, handle_event/3, handle_sync_event/4, handle_info/3, terminate/3, code_change/4,
% State functions
waiting_for_players/2, in_progress/2, in_progress/3
]).
%% Utility functions
-export([get_number/1, add_player/2, get_active_player/1,
shut_down/1
]).
%%% State Record
-record(?MODULE, {
number,
players = [],
active_player,
game = {0,0,0,0,0,0,0,0,0},
turn =0
}).
%%% Public API
%% Behavior functions
start(Number) ->
gen_fsm:start(?MODULE, [Number], []).
start_link(Number) ->
gen_fsm:start_link(?MODULE, [Number], []).
%% Behavior callbacks
init([Number]) ->
random:seed(erlang:now()),
{ok, waiting_for_players, #?MODULE {number = Number}}.
handle_event(stop, _StateName, Data) ->
{stop, normal, Data};
handle_event(Event, StateName, Data) ->
unexpected(Event, StateName, Data),
{next_state, StateName, Data}.
handle_sync_event(getnumber, _From, StateName, Data) ->
{reply, Data#?MODULE.number, StateName, Data};
handle_sync_event(getactiveplayer, _From, StateName, Data) ->
{reply, Data#?MODULE.active_player, StateName, Data};
handle_sync_event(Event, _From, StateName, Data) ->
unexpected(Event, StateName, Data),
{next_state, StateName, Data}.
handle_info(Info, StateName, Data) ->
unexpected(Info, StateName, Data),
{next_state, StateName, Data}.
code_change(_OldVsn, StateName, Data, _Extra) ->
{ok, StateName, Data}.
terminate(normal, StateName, State=#?MODULE{}) ->
io:format("Game terminated normally, state name = ~p, ~p~n", [StateName, State]);
terminate(Reason, StateName, State) ->
io:format("Game terminated abnormally, reason = ~p, state name = ~p, state = ~p~n", [Reason, StateName, State]).
%% State functions
waiting_for_players({addplayer, Player}, Data) ->
Game = Data#?MODULE.number,
io:format("Game ~p, adding player ~p~n", [Game, player:get_name(Player)]),
NewPlayers = [Player | Data#?MODULE.players],
PlayerCount = length(NewPlayers),
gen_fsm:send_event(Player, {gamejoined, self()}),
AP = maybe_play(PlayerCount,NewPlayers,Data#?MODULE.game),
{next_state, waiting_for_players(PlayerCount), Data#?MODULE{players = NewPlayers, active_player = AP}};
waiting_for_players(Event, State) ->
io:format("waiting_for_tournament: received unknown event ~p, state = ~p~n", [Event, State]), {next_state, waiting_for_tournament, State}.
in_progress({playeraction, Player, {Row, Column}}, State = #?MODULE{active_player = Player, game = Game , players = Players, turn = Turn}) ->
io:format("in_progress: Player ~p did ~p~n", [Player, {Row, Column}]),
Index = index(Column,Row),
case is_valid_action(Index,Game) of
true ->
_NewGame = setelement(Index,Game,Player),
_CheckWinner = check_winner(_NewGame,Turn+1),
(game@homepc)1> net_adm:ping(p2@homepc).
pong
(game@homepc)2>
(game@homepc)2> net_adm:ping(p1@homepc).
pong
(game@homepc)3>
(game@homepc)3> {ok,G} = game:start(5582).
{ok,<0.50.0>}
(game@homepc)4>
(game@homepc)4> {shell,p2@homepc} ! G.
<0.50.0>
(game@homepc)5>
(game@homepc)5>
(game@homepc)5> {shell,p1@homepc} ! G.
<0.50.0>
(game@homepc)6>
(game@homepc)6>
Game 5582, adding player "Bob"
Game 5582, adding player "Joe"
in_progress: Player <7169.52.0> did {2,2}
in_progress: Player <6080.52.0> did {2,2}
in_progress: Player <6080.52.0> did {2,8}
in_progress: Player <6080.52.0> did {1.0,1}
in_progress: Player <6080.52.0> did {1,1}
in_progress: Player <7169.52.0> did {1,3}
in_progress: Player <6080.52.0> did {3,1}
in_progress: Player <7169.52.0> did {2,1}
in_progress: Player <6080.52.0> did {3,3}
in_progress: Player <7169.52.0> did {2,3}
Game terminated normally, state name = in_progress, {game,5582,
[<6080.52.0>,<7169.52.0>],
<7169.52.0>,
{<6080.52.0>,0,
<7169.52.0>,<7169.52.0>,
<7169.52.0>,0,
<6080.52.0>,0,
<6080.52.0>},
6}
(game@homepc)6>(p1@homepc)1> register(shell,self()).
true
(p1@homepc)2>
(p1@homepc)2>
(p1@homepc)2> G = receive M -> M end.
<5980.50.0>
(p1@homepc)3>
(p1@homepc)3> {ok,P1} = player:start("Bob").
{ok,<0.52.0>}
(p1@homepc)4>
(p1@homepc)4> game:add_player(G, P1).
ok
(p1@homepc)5>
Player Bob: joined game 5582/<5980.50.0>
Bob in_game: it is your turn to play
+-+-+-+
| | | |
+-+-+-+
| | | |
+-+-+-+
| | | |
+-+-+-+
(p1@homepc)5> player:make_move(P1, 2, 2).
in_game: setting row 2, column 2
ok
movement accepted
Bob in_game: it is your turn to play
+-+-+-+
|O| | |
+-+-+-+
| |X| |
+-+-+-+
| | | |
+-+-+-+
(p1@homepc)6> player:make_move(P1, 1, 3).
in_game: setting row 1, column 3
ok
movement accepted
Bob in_game: it is your turn to play
+-+-+-+
|O| |X|
+-+-+-+
| |X| |
+-+-+-+
|O| | |
+-+-+-+
(p1@homepc)7> player:make_move(P1, 2, 1).
in_game: setting row 2, column 1
ok
movement accepted
Bob in_game: it is your turn to play
+-+-+-+
|O| |X|
+-+-+-+
|X|X| |
+-+-+-+
|O| |O|
+-+-+-+
(p1@homepc)8> player:make_move(P1, 2, 3).
in_game: setting row 2, column 3
ok
Bob: <5980.50.0> Game over, you won
(p1@homepc)9>(p2@homepc)1> register(shell,self()).
true
(p2@homepc)2>
(p2@homepc)2>
(p2@homepc)2> G = receive M -> M end.
<5980.50.0>
(p2@homepc)3>
(p2@homepc)3> {ok,P2} = player:start("Joe").
{ok,<0.52.0>}
(p2@homepc)4>
(p2@homepc)4> game:add_player(G, P2).
ok
(p2@homepc)5>
Player Joe: joined game 5582/<5980.50.0>
(p2@homepc)5> player:make_move(P2, 2, 2).
in_game: setting row 2, column 2
ok
Please Joe wait for your turn
Joe in_game: it is your turn to play
+-+-+-+
| | | |
+-+-+-+
| |O| |
+-+-+-+
| | | |
+-+-+-+
(p2@homepc)6> player:make_move(P2, 2, 2).
in_game: setting row 2, column 2
ok
invalid movement {2,2}
(p2@homepc)7> player:make_move(P2, 2, 8).
in_game: setting row 2, column 8
ok
invalid movement {2,8}
(p2@homepc)8> player:make_move(P2, 1.0 , 1).
in_game: setting row 1.0, column 1
ok
invalid movement {1.0,1}
(p2@homepc)9> player:make_move(P2, 1, 1).
in_game: setting row 1, column 1
ok
movement accepted
Joe in_game: it is your turn to play
+-+-+-+
|X| |O|
+-+-+-+
| |O| |
+-+-+-+
| | | |
+-+-+-+
(p2@homepc)10> player:make_move(P2, 3, 1).
in_game: setting row 3, column 1
ok
movement accepted
Joe in_game: it is your turn to play
+-+-+-+
|X| |O|
+-+-+-+
|O|O| |
+-+-+-+
|X| | |
+-+-+-+
(p2@homepc)11> player:make_move(P2, 3, 3).
in_game: setting row 3, column 3
ok
movement accepted
Joe: <5980.50.0> Game over, you loose
(p2@homepc)12>Context
StackExchange Code Review Q#74216, answer score: 2
Revisions (0)
No revisions yet.