refactor playerControl

Allow to propagate errors in order to handle them acordingly.
Use playerNotifications by value to reduce the amount of allocations.
Add session managment playerNotifications.
Support marshalling notifications to plain text.

8 files changed, 286 insertions, 43 deletions

diff --git a/go/activities/draft.go b/go/activities/draft.go
index 85389106..e326050d 100644
--- a/go/activities/draft.go
+++ b/go/activities/draft.go
@@ -109,7 +109,7 @@ func NewLocalDraft(width, height int, playerName string) *Draft {
 	return d
 }
 
-func StartNewRemoteDraft(width, height int, ctrl game.PlayerControl, firstPack *game.PlayerNotification) *Draft {
+func StartNewRemoteDraft(width, height int, ctrl game.PlayerControl, firstPack game.PlayerNotification) *Draft {
 	d := &Draft{
 		Collection: ui.Collection{Width: width, Height: height},
 		player:     ctrl.Player(),
@@ -121,7 +121,12 @@ func StartNewRemoteDraft(width, height int, ctrl game.PlayerControl, firstPack *
 }
 
 func (d *Draft) Update() error {
-	if n := d.ctrl.RecvNotification(); n != nil {
+	n, err := d.ctrl.RecvNotification()
+	if err != nil {
+		// FIXME
+	}
+
+	if n.Valid() {
 		log.Println("Received draft pick: %v\n", n)
 		d.presentPack(n)
 	}
@@ -141,7 +146,7 @@ func (d *Draft) Update() error {
 	return nil
 }
 
-func (d *Draft) presentPack(n *game.PlayerNotification) {
+func (d *Draft) presentPack(n game.PlayerNotification) {
 	pack := n.Context.(game.PileOfCards)
 	grid := ui.NewCardGrid(0, 0, d.Width-SEALED_DECK_STRIP_WIDTH, d.Height, 0.5, pack).Columns(3)
 	selectCard := func(x, y int) {
diff --git a/go/client/game.go b/go/client/game.go
index c0724547..7c54ac0f 100644
--- a/go/client/game.go
+++ b/go/client/game.go
@@ -449,8 +449,11 @@ func (g *Game) passPriority() {
 }
 
 func (g *Game) handlePlayerNotifications() {
-	n := g.playerCtrl.RecvNotification()
-	for n != nil {
+	n, err := g.playerCtrl.RecvNotification()
+	if err != nil {
+		// FIXME
+	}
+	for n.Valid() {
 		log.Println("Received", n)
 		switch n.Notification {
 		case game.PriorityNotification:
@@ -493,7 +496,10 @@ func (g *Game) handlePlayerNotifications() {
 		g.stateBar.ForceRedraw()
 		g.handLayer.ForceRedraw()
 
-		n = g.playerCtrl.RecvNotification()
+		n, err = g.playerCtrl.RecvNotification()
+		if err != nil {
+			// FIXME
+		}
 	}
 }
 
diff --git a/go/game/draft.go b/go/game/draft.go
index 2b554a82..f180e0ca 100644
--- a/go/game/draft.go
+++ b/go/game/draft.go
@@ -105,19 +105,19 @@ func (ctrl *randomDraftAiCtrl) Player() *Player {
 func (ctrl *randomDraftAiCtrl) Close() {
 }
 
-func (ctrl *randomDraftAiCtrl) SendNotification(n PlayerNotification) {
+func (ctrl *randomDraftAiCtrl) SendNotification(n PlayerNotification) error {
 	ctrl.pack = n.Context.(PileOfCards)
+	return nil
 }
 
-func (ctrl *randomDraftAiCtrl) RecvAction() Action {
-	availCards := ctrl.pack.Cards()
-	var pick int
-	if n := len(availCards); n > 1 {
-		pick = rand.Intn(n - 1)
-	} else {
-		pick = 0
-	}
-	return NewDraftPick(ctrl.ai, ctrl.pack, availCards[pick])
+func (ctrl *randomDraftAiCtrl) RecvNotification() (n PlayerNotification, err error) { return }
+
+func (ctrl *randomDraftAiCtrl) SendAction(Action) error {
+	return nil
+}
+
+func (ctrl *randomDraftAiCtrl) RecvAction() (Action, error) {
+	return NewDraftPick(ctrl.ai, ctrl.pack, ctrl.pack.RandomCard()), nil
 }
 
 func (d *Draft) AddRandomAi() {
diff --git a/go/game/kraken.go b/go/game/kraken.go
index 724980df..94d63898 100644
--- a/go/game/kraken.go
+++ b/go/game/kraken.go
@@ -82,6 +82,15 @@ func (c *KrakenControl) Player() *Player {
 	return c.kraken
 }
 
+func (c *KrakenControl) SendAction(Action) error {
+	return nil
+}
+
+func (c *KrakenControl) RecvNotification() (PlayerNotification, error) {
+	var n PlayerNotification
+	return n, nil
+}
+
 func (c *KrakenControl) Close() {
 	close(c.actions)
 }
@@ -156,20 +165,21 @@ func (ctrl *KrakenControl) krakenTurn() {
 	ctrl.actions <- NewPassPriority(ctrl.kraken)
 }
 
-func (ctrl *KrakenControl) RecvAction() Action {
+func (ctrl *KrakenControl) RecvAction() (Action, error) {
 	s := ctrl.kraken.gameState
 	kraken := ctrl.kraken
 	if !s.IsActivePlayer(kraken) ||
 		s.activePhase != Phases.ActionPhase || !s.stack.IsEmpty() {
 
-		return NewPassPriority(ctrl.kraken)
+		return NewPassPriority(ctrl.kraken), nil
 	}
 
 	ctrl.syncGameState.Done()
-	return <-ctrl.actions
+	return <-ctrl.actions, nil
 }
 
-func (ctrl *KrakenControl) SendNotification(n PlayerNotification) {
+func (ctrl *KrakenControl) SendNotification(n PlayerNotification) error {
+	return nil
 }
 
 func (p *Player) IsKraken() bool {
diff --git a/go/game/player.go b/go/game/player.go
index 96b95ea5..974223d1 100644
--- a/go/game/player.go
+++ b/go/game/player.go
@@ -120,19 +120,26 @@ func (p *Player) upkeep() {
 		return
 	}
 
-	a := prompt(p.Ctrl, newUpkeepPrompt(p))
+	a, err := prompt(p.Ctrl, newUpkeepPrompt(p))
+	if err != nil {
+		// FIXME: handle error
+	}
 
 	if _, ok := a.(*PassPriority); ok {
 		a = newUpkeepAction(p)
 	}
 
+	// FIXME: upkeep action is special paying is part of its effect
 	p.gameState.declareAction(a)
 	p.gameState.stack.resolve()
 }
 
 func (p *Player) actionPhase() {
 	for {
-		a := promptAction(p.Ctrl)
+		a, err := promptAction(p.Ctrl)
+		if err != nil {
+			// FIXME
+		}
 		if _, ok := a.(*PassPriority); ok {
 			return
 		}
@@ -146,7 +153,10 @@ func (p *Player) buyPhase() {
 	if p.Store.Size() == 0 && s.Map().HasStores() && len(p.knownStores) == 0 {
 		return
 	}
-	a := promptBuy(p.Ctrl)
+	a, err := promptBuy(p.Ctrl)
+	if err != nil {
+		// FIXME
+	}
 
 	if _, ok := a.(*PassPriority); ok {
 		return
@@ -181,7 +191,12 @@ func (p *Player) discardStep() {
 
 func (p *Player) PromptHandCardSelection(min, max int) []*Card {
 	p.Ctrl.SendNotification(newHandCardSelectionPrompt(p, min, max))
-	switch a := p.Ctrl.RecvAction().(type) {
+	_a, err := p.Ctrl.RecvAction()
+	if err != nil {
+		// FIXME
+	}
+
+	switch a := _a.(type) {
 	case *TargetSelection:
 		err := a.CheckTargets(p.gameState)
 		if err != nil {
diff --git a/go/game/playerControl.go b/go/game/playerControl.go
index c472f4f0..74af6702 100644
--- a/go/game/playerControl.go
+++ b/go/game/playerControl.go
@@ -3,21 +3,27 @@ package game
 import (
 	"fmt"
 	"log"
+	"regexp"
 )
 
 type PlayerNotificationType int
 
 const (
-	DeclaredActionNotification PlayerNotificationType = iota
+	InvalidNotification PlayerNotificationType = iota
+	DeclaredActionNotification
 	ResolvedActionNotification
 	PriorityNotification
 	TargetSelectionPrompt
 	DeclareTriggeredActionsPrompt
 	DraftPickPrompt
+	JoinedPlayerNotification
+	ReadyPlayerNotification
 )
 
 func (n PlayerNotificationType) String() string {
 	switch n {
+	case InvalidNotification:
+		return "InvalidNotification"
 	case DeclaredActionNotification:
 		return "DeclaredActionNotification"
 	case ResolvedActionNotification:
@@ -30,6 +36,10 @@ func (n PlayerNotificationType) String() string {
 		return "DeclareTriggeredActionsPrompt"
 	case DraftPickPrompt:
 		return "DraftPickPrompt"
+	case JoinedPlayerNotification:
+		return "JoinedPlayerNotification"
+	case ReadyPlayerNotification:
+		return "ReadyPlayerNotification"
 	default:
 		log.Panicf("Unhandled notification %d", n)
 		return ""
@@ -46,7 +56,7 @@ type PlayerNotification struct {
 	Error        error
 }
 
-func (n *PlayerNotification) String() string {
+func (n PlayerNotification) String() string {
 	if n.Error != nil {
 		return fmt.Sprintf("error %v: %v", n.Notification, n.Error)
 	}
@@ -54,6 +64,10 @@ func (n *PlayerNotification) String() string {
 	return fmt.Sprintf("%v: %v", n.Notification, n.Context)
 }
 
+func (n PlayerNotification) Valid() bool {
+	return n.Notification != InvalidNotification
+}
+
 type TargetSelectionCtx struct {
 	Action Action
 	Prompt string
@@ -100,30 +114,158 @@ func newDraftPickPrompt(pack PileOfCards) PlayerNotification {
 	return PlayerNotification{DraftPickPrompt, pack, nil}
 }
 
+// Create a notification about a joined new player.
+// The notification only contains the name since a full `game.Player` may not be
+// reconstructable for clients without a game state (e.g. draft client).
+func NewJoinedPlayerNotification(p *Player) PlayerNotification {
+	return PlayerNotification{JoinedPlayerNotification, p.Name, nil}
+}
+
+// Create a notification about a player's readiness.
+// The notification only contains the name since a full `game.Player` may not be
+// reconstructable for clients without a game state (e.g. draft client).
+func NewReadyPlayerNotification(p *Player) PlayerNotification {
+	return PlayerNotification{ReadyPlayerNotification, p.Name, nil}
+}
+
+// Marshal marshals a PlayerNotification to plain text.
+//
+// The marshaled PlayerNotification has the form `!TYPE{CONTEXT}`.
+func (n PlayerNotification) Marshal(s State) []byte {
+	out := []byte{'!'}
+	ctx := []byte{}
+	switch n.Notification {
+	case DraftPickPrompt:
+		out = append(out, []byte("pick")...)
+		ctx = []byte(n.Context.(PileOfCards).String())
+	case PriorityNotification:
+		out = append(out, []byte("priority")...)
+	case DeclaredActionNotification:
+		out = append(out, []byte("declared")...)
+		ctx = MarshalAction(n.Context.(Action))
+	case ResolvedActionNotification:
+		out = append(out, []byte("resolved")...)
+		if n.Error != nil {
+			ctx = []byte(n.Error.Error())
+		}
+	case JoinedPlayerNotification:
+		out = append(out, []byte("joined")...)
+		ctx = []byte(n.Context.(string))
+	case ReadyPlayerNotification:
+		out = append(out, []byte("ready")...)
+		ctx = []byte(n.Context.(string))
+	default:
+		log.Fatalf("Marshal(%s) not implement\n", n)
+	}
+	out = fmt.Appendf(out, "{%s}", ctx)
+
+	return out
+}
+
+var PlayerNotificationRegex = regexp.MustCompile(`!(.*)\{(.*)\}`)
+
+type InvalidPlayerNotificationError struct {
+	ErrorString string
+}
+
+func (err InvalidPlayerNotificationError) Error() string {
+	return err.ErrorString
+}
+
+var (
+	ErrBadFormat           = InvalidPlayerNotificationError{"bad format"}
+	ErrUnknownNotification = InvalidPlayerNotificationError{"unknown notification"}
+	ErrNoContextExpected   = InvalidPlayerNotificationError{"no context expected"}
+)
+
+// UnmarshalPlayerNotification creates a UnmarshalPlayerNotification from plain text.
+// For the PlayerNotification plain text format see MarshalPlayerNotification.
+func UnmarshalPlayerNotification(s State, in []byte) (n PlayerNotification, err error) {
+	m := PlayerNotificationRegex.FindSubmatch(in)
+	if len(m) != 3 {
+		return n, ErrBadFormat
+	}
+	t := string(m[1])
+	ctx := m[2]
+
+	switch t {
+	case "pick":
+		pack := NewPileOfCards()
+		err = pack.FromString(string(ctx))
+		if err != nil {
+			return
+		}
+		n = newDraftPickPrompt(pack)
+
+	case "priority":
+		if len(ctx) > 0 {
+			err = ErrNoContextExpected
+		}
+		n = newPriorityNotification()
+
+	case "resolved":
+		var err error
+		if len(ctx) > 0 {
+			err = ErrNoContextExpected
+		}
+		n = newResolvedActionNotification(s.Stack().Actions[len(s.Stack().Actions)-1], err)
+
+	case "joined":
+		n = PlayerNotification{JoinedPlayerNotification, string(ctx), nil}
+
+	case "ready":
+		n = PlayerNotification{ReadyPlayerNotification, string(ctx), nil}
+	default:
+		return n, ErrUnknownNotification
+	}
+	return
+}
+
 type PlayerControl interface {
 	Player() *Player
-	RecvAction() Action
-	SendNotification(PlayerNotification)
+	// SendAction sends an Action and blocks until it is sent.
+	SendAction(Action) error
+	// RecvAction returns the next Action.
+	// It blocks until the Action is received or an error occurs.
+	RecvAction() (Action, error)
+	// SendNotification sends an PlayerNotification and blocks until it is sent.
+	SendNotification(PlayerNotification) error
+	// RecvNotification returns a new PlayerNotification.
+	// If no new notification is available an zero/invalid PlayerNotification is returned and err == nil.
+	// RecvNotification never blocks.
+	RecvNotification() (PlayerNotification, error)
 	Close()
 }
 
+// ChanPlayerControl implements a PlayerControl using two go channels.
+// Objects are transferred directly.
+// Therefore ChanPlayerControl is only suitable for communication in the same process.
 type ChanPlayerControl struct {
 	player        *Player
 	actions       chan Action
 	notifications chan PlayerNotification
 }
 
-func (c *ChanPlayerControl) Player() *Player                       { return c.player }
-func (c *ChanPlayerControl) SendAction(a Action)                   { c.actions <- a }
-func (c *ChanPlayerControl) RecvAction() Action                    { return <-c.actions }
-func (c *ChanPlayerControl) SendNotification(n PlayerNotification) { c.notifications <- n }
-func (c *ChanPlayerControl) RecvNotification() *PlayerNotification {
+func (c *ChanPlayerControl) Player() *Player { return c.player }
+func (c *ChanPlayerControl) SendAction(a Action) error {
+	c.actions <- a
+	return nil
+}
+func (c *ChanPlayerControl) RecvAction() (Action, error) {
+	return <-c.actions, nil
+}
+func (c *ChanPlayerControl) SendNotification(n PlayerNotification) error {
+	c.notifications <- n
+	return nil
+}
+
+func (c *ChanPlayerControl) RecvNotification() (PlayerNotification, error) {
+	var n PlayerNotification
 	select {
-	case n := <-c.notifications:
-		return &n
+	case n = <-c.notifications:
 	default:
-		return nil
 	}
+	return n, nil
 }
 func (c *ChanPlayerControl) Close() {
 	close(c.actions)
@@ -136,15 +278,15 @@ func NewChanPlayerControl(p *Player) *ChanPlayerControl {
 	return &ChanPlayerControl{p, a, n}
 }
 
-func prompt(ctrl PlayerControl, notification PlayerNotification) Action {
+func prompt(ctrl PlayerControl, notification PlayerNotification) (Action, error) {
 	ctrl.SendNotification(notification)
 	return ctrl.RecvAction()
 }
 
-func promptBuy(ctrl PlayerControl) Action {
+func promptBuy(ctrl PlayerControl) (Action, error) {
 	return prompt(ctrl, newBuyPrompt(ctrl.Player()))
 }
 
-func promptAction(ctrl PlayerControl) Action {
+func promptAction(ctrl PlayerControl) (Action, error) {
 	return prompt(ctrl, newPriorityNotification())
 }
diff --git a/go/game/playerControl_test.go b/go/game/playerControl_test.go
new file mode 100644
index 00000000..34f20c59
--- /dev/null
+++ b/go/game/playerControl_test.go
@@ -0,0 +1,56 @@
+package game
+
+import (
+	"testing"
+)
+
+const PRORITY_STRING = "!priority{}"
+
+func TestMarshalPriority(t *testing.T) {
+	exp := PRORITY_STRING
+	n := newPriorityNotification()
+	is := string(n.Marshal(nil))
+	if is != exp {
+		t.Fatalf("expected string %s does not match %s\n", exp, is)
+	}
+}
+
+func TestUnmarshalPriority(t *testing.T) {
+	in := PRORITY_STRING
+	n, err := UnmarshalPlayerNotification(nil, []byte(in))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if n.Notification != PriorityNotification {
+		t.Fatalf("expexted PriorityNotificaton not %s", n.Notification)
+	}
+}
+
+const PICK_STRING = "!pick{[base/archer, magic/more!]}"
+
+func TestMarshalPick(t *testing.T) {
+	exp := PICK_STRING
+	poc := NewPileOfCards()
+	poc.AddCard(NewCard("base/archer"))
+	poc.AddCard(NewCard("magic/more!"))
+	n := newDraftPickPrompt(poc)
+	is := string(n.Marshal(nil))
+	if is != exp {
+		t.Fatalf("expected string %s does not match %s\n", exp, is)
+	}
+}
+
+func TestUnmarshalPick(t *testing.T) {
+	in := PICK_STRING
+	n, err := UnmarshalPlayerNotification(nil, []byte(in))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if n.Notification != DraftPickPrompt {
+		t.Fatalf("expexted DraftPickPrompt not %s", n.Notification)
+	}
+	pack := n.Context.(PileOfCards)
+	if len(pack.Cards()) != 2 {
+		t.Fatalf("expexted two cards in pack not %d", len(pack.Cards()))
+	}
+}
diff --git a/go/game/state.go b/go/game/state.go
index 6fa8b4c3..25385f31 100644
--- a/go/game/state.go
+++ b/go/game/state.go
@@ -468,14 +468,19 @@ func (s *LocalState) orderTriggeredActions(triggeredActions []*TriggeredAction)
 
 	orderedActions := []Action{}
 	nPlayers := len(s.players)
-	for i := 0; i < nPlayers; i++ {
+	for i := range nPlayers {
 		p := s.players[(i+s.activePlayerId-1)%nPlayers]
 
 		actions := playerActions[p]
 		pActions := make([]Action, 0, len(actions))
 		p.Ctrl.SendNotification(newDeclareTriggeredActionsPrompt(actions))
 
-		switch a := p.Ctrl.RecvAction().(type) {
+		_a, err := p.Ctrl.RecvAction()
+		if err != nil {
+			// FIXME
+		}
+
+		switch a := _a.(type) {
 		case *DeclareTriggeredActionsAction:
 			for _, a := range a.actions { // Use the order passed back by the user
 				pActions = append(pActions, a)
@@ -504,8 +509,12 @@ func (s *LocalState) allPassing(skipFirst bool) bool {
 		p := s.players[(i+s.activePlayerId-1)%nPlayers]
 		s.stateBasedActions()
 		p.Ctrl.SendNotification(newPriorityNotification())
-		a := p.Ctrl.RecvAction()
-		switch a.(type) {
+		_a, err := p.Ctrl.RecvAction()
+		if err != nil {
+			// FIXME
+		}
+
+		switch a := _a.(type) {
 		case *PassPriority:
 			log.Printf("%s passed %d/%d\n", p.Name, i+1, nPlayers)
 		default: