Added server side grace period support. #174
Conversation
|
Also, this is not yet tested - I wanted to get a sanity check by visual inspection first. |
| p.Phase.Host, | ||
| ); err != nil { | ||
| log.Errorf(p.Context, "AsyncSendMsgFunc(..., %v, %v, %+v, %q, %q): %v; fix it?", p.Phase.GameID, DiplicitySender, p.Variant.Nations, notificationBody, p.Phase.Host, err) | ||
| return false, err |
There was a problem hiding this comment.
I'm not super keen on these error conditions returning false. The grace period has been used up and the deadline has been postponed, so I think this should return true, even though the messages weren't sent.
There was a problem hiding this comment.
Go custom is that if the error is non-nil, then everything else should be ignored. The "false" is just because we need something to return, and false is the default boolean value.
Also - the error will cause the wrapping transaction to roll back, and thus no grace period will actually be used.
|
The logic looks right to me. This will allow a player to use all their grace periods on the same phase, but I think that's ok. |
game/phase.go
Outdated
| allMembers = append(allMembers, string(nat)) | ||
| } | ||
| notificationBody := fmt.Sprintf( | ||
| "%v weren't ready to resolve and gave no orders, since they still have grace periods phase resolution has been postponed %v (until %v).", |
There was a problem hiding this comment.
The English will still be a little off here for a single nation: "England weren't ready"
There was a problem hiding this comment.
%v missed the last deadline. They are using one of their grace periods of %v. The new deadline for this phase will be %v.
There was a problem hiding this comment.
I think that sounds better and explains it a bit clearer. It also works for all nations/variants.
There was a problem hiding this comment.
Problem is that multiple users will be using their grace periods. How would you phrase that?
There was a problem hiding this comment.
@tttppp I changed the pluralization to be correct.
|
We should have a way to know in the client whether we're in a grace period. I think we keep the current phase, so it would be "Spring 1902 (grace)". We do need:
|
… grace, and preventing grace usage when it is set to true.
I added Phase.GraceUsed which will show this.
I don't understand - when grace period is used the current phase will just be extended.
Yes, exactly.
This is in the PR now.
Why? You are not committed and need to commit, isn't that enough? |
GracePeriodMinutesandGracePeriodsPerPlayer.GracePeriodsUsed.GraceUsed.GracePeriodMinutes> 0,GracePeriodsPerPlayer> 0, latest phase!GraceUsed, and at least one player with no orders, and not ready to resolve hasGracePeriodsUsed<GracePeriodsPerPlayerthey will have theirGracePeriodsUsedbumped, the current phase will haveGraceUsedset to true, be postponed, and the late players will be named and shamed in a message to the conference channel.@JorenC, @tttppp, @johnpooch PTAL