From 028c2eaf83ed2c21a935a771ce73860f26cbd666 Mon Sep 17 00:00:00 2001 From: Anton Tarasenko Date: Wed, 22 Mar 2023 01:54:01 +0700 Subject: [PATCH] Fix documentation and naming for `VotingModel` --- .../API/Commands/Voting/TEST_Voting.uc | 8 +- .../API/Commands/Voting/VotingModel.uc | 77 +++++++++---------- 2 files changed, 41 insertions(+), 44 deletions(-) diff --git a/sources/BaseAPI/API/Commands/Voting/TEST_Voting.uc b/sources/BaseAPI/API/Commands/Voting/TEST_Voting.uc index c258303..e4df93a 100644 --- a/sources/BaseAPI/API/Commands/Voting/TEST_Voting.uc +++ b/sources/BaseAPI/API/Commands/Voting/TEST_Voting.uc @@ -115,7 +115,7 @@ protected static function MakeFaultyYesVote( id = UserID(__().memory.Allocate(class'UserID')); id.Initialize(__().text.FromString(voterID)); Issue("Illegal vote had unexpected result."); - TEST_ExpectTrue(model.AddVote(id, true) == expected); + TEST_ExpectTrue(model.CastVote(id, true) == expected); } protected static function MakeFaultyNoVote( @@ -127,7 +127,7 @@ protected static function MakeFaultyNoVote( id = UserID(__().memory.Allocate(class'UserID')); id.Initialize(__().text.FromString(voterID)); Issue("Illegal vote had unexpected result."); - TEST_ExpectTrue(model.AddVote(id, false) == expected); + TEST_ExpectTrue(model.CastVote(id, false) == expected); } protected static function VoteYes(VotingModel model, string voterID, ExpectedOutcome expected) { @@ -136,7 +136,7 @@ protected static function VoteYes(VotingModel model, string voterID, ExpectedOut id = UserID(__().memory.Allocate(class'UserID')); id.Initialize(__().text.FromString(voterID)); Issue("Failed to add legitimate vote."); - TEST_ExpectTrue(model.AddVote(id, true) == VFR_Success); + TEST_ExpectTrue(model.CastVote(id, true) == VFR_Success); if (expected == TEST_EO_Continue) { Issue("Vote, that shouldn't have ended voting, ended it."); TEST_ExpectTrue(model.GetStatus() == VPM_InProgress); @@ -155,7 +155,7 @@ protected static function VoteNo(VotingModel model, string voterID, ExpectedOutc id = UserID(__().memory.Allocate(class'UserID')); id.Initialize(__().text.FromString(voterID)); Issue("Failed to add legitimate vote."); - TEST_ExpectTrue(model.AddVote(id, false) == VFR_Success); + TEST_ExpectTrue(model.CastVote(id, false) == VFR_Success); if (expected == TEST_EO_Continue) { Issue("Vote, that shouldn't have ended voting, ended it."); TEST_ExpectTrue(model.GetStatus() == VPM_InProgress); diff --git a/sources/BaseAPI/API/Commands/Voting/VotingModel.uc b/sources/BaseAPI/API/Commands/Voting/VotingModel.uc index 20870f8..a4e2469 100644 --- a/sources/BaseAPI/API/Commands/Voting/VotingModel.uc +++ b/sources/BaseAPI/API/Commands/Voting/VotingModel.uc @@ -21,52 +21,46 @@ */ class VotingModel extends AcediaObject; -//! Class for counting votes according to voting policies it was configured with. +//! This class counts votes according to the configured voting policies. //! -//! Its main purpose is to separate voting logic from the voting interface, simplifying its -//! implementation and making actual logic easily testable. +//! Its main purpose is to separate the voting logic from the voting interface, making +//! the implementation simpler and the logic easier to test. //! //! # Usage //! -//! 1. Allocate instance of [`VotingModel`]; -//! 2. Call [`Initialize()`] to set required policies; +//! 1. Allocate an instance of the [`VotingModel`] class. +//! 2. Call [`Initialize()`] to set the required policies. //! 3. Use [`UpdatePotentialVoters()`] to specify which users are allowed to vote. -//! This set can be changed at any point in time later. -//! How votes will be recounted will depend on policies set in previous [`Initialize()`] call. -//! 4. Use [`AddVote()`] to add a vote from users. -//! 5. Check [`GetStatus()`] after either [`UpdatePotentialVoters()`] or [`AddVote()`] calls to -//! check whether voting has concluded. -//! You can release [`VotingModel`]'s reference after you've gotten the result, since once -//! voting has concluded, its result cannot be changed. +//! You can change this set at any time. The method used to recount the votes will depend on +//! the policies set during the previous [`Initialize()`] call. +//! 4. Use [`CastVote()`] to add a vote from a user. +//! 5. After calling either [`UpdatePotentialVoters()`] or [`CastVote()`], check [`GetStatus()`] to +//! see if the voting has concluded. +//! Once voting has concluded, the result cannot be changed, so you can release the reference +//! to the [`VotingModel`] object. -/// Describes how [`VotingModel`] must react to user making potentially illegal reactions. +/// Describes how [`VotingModel`] should react when a user performs potentially illegal actions. /// /// Illegal here means that either corresponding operation won't be permitted or any vote made /// would be considered invalid. +/// +/// Leaving means simply no longer being in a potential pool of voters, which includes actually +/// leaving the game and simply losing rights to vote. enum VotingPolicies { - /// Consider illegal anything that can be illegal. + /// Anything that can be considered illegal actions is prohibited. /// - /// Leaving during voting will make a vote invalid. - /// Leaving means losing rights to vote. + /// Leaving (or losing rights to vote) during voting will make a vote invalid. /// /// Changing vote is forbidden. VP_Restrictive, - /// Leaving during voting is legal, everything else potentially illegal is still illegal. - /// - /// Changing vote is forbidden. + /// Leaving during voting is allowed. Changing a vote is not allowed. VP_CanLeave, - /// Changing one's vote is allowed, everything else potentially illegal is still illegal. - /// - /// Leaving during voting will make a vote invalid. - /// Leaving means losing rights to vote. + /// Changing one's vote is allowed. If a user leaves during voting, their vote will be invalid. VP_CanChangeVote, - /// Leaving during voting and changing a vote is allowed, everything else potentially illegal is - /// still illegal. - /// - /// Leaving means losing rights to vote. + /// Leaving during voting and changing a vote is allowed. Leaving means losing rights to vote. /// - /// Currently this flag means that every option it provides is allowed, but this might change in - /// the future if more options would be added. + /// Currently, this policy allows all available options, but this may change in the future if + /// more options are added. VP_CanLeaveAndChangeVote }; @@ -150,7 +144,7 @@ public final function Initialize(VotingPolicies policies) { /// Returns current status of voting. /// -/// This method should be checked after both [`AddVote()`] and [`UpdatePotentialVoters()`] to check +/// This method should be checked after both [`CastVote()`] and [`UpdatePotentialVoters()`] to check /// whether either of them was enough to conclude the voting result. public final function VotingModelStatus GetStatus() { return status; @@ -178,19 +172,19 @@ public final function UpdatePotentialVoters(array potentialVoters) { /// /// Adding a vote can fail if [`voter`] isn't allowed to vote or has already voted and policies /// forbid changing that vote. -public final function VotingResult AddVote(UserID voter, bool forSuccess) { +public final function VotingResult CastVote(UserID voter, bool voteForSuccess) { local bool votesSameWay; local PlayerVoteStatus currentVote; if (status != VPM_InProgress) { return VFR_VotingEnded; } - if (!AllowedToVote(voter)) { + if (!IsVotingAllowedFor(voter)) { return VFR_NotAllowed; } currentVote = HasVoted(voter); - votesSameWay = (forSuccess && currentVote == PVS_VoteFor) - || (!forSuccess && currentVote == PVS_VoteAgainst); + votesSameWay = (voteForSuccess && currentVote == PVS_VoteFor) + || (!voteForSuccess && currentVote == PVS_VoteAgainst); if (votesSameWay) { return VFR_AlreadyVoted; } @@ -199,7 +193,7 @@ public final function VotingResult AddVote(UserID voter, bool forSuccess) { } EraseVote(voter); voter.NewRef(); - if (forSuccess) { + if (voteForSuccess) { votesFor[votesFor.length] = voter; } else { votesAgainst[votesAgainst.length] = voter; @@ -208,12 +202,15 @@ public final function VotingResult AddVote(UserID voter, bool forSuccess) { return VFR_Success; } -/// Checks if provided user is allowed to vote. +/// Checks if the provided user is allowed to vote based on the current list of potential voters. +/// +/// The right to vote is decided solely by the list of potential voters set using +/// [`UpdatePotentialVoters()`]. +/// However, even if a user is on the list of potential voters, they may not be allowed to vote if +/// they have already cast a vote and the voting policies do not allow vote changes. /// -/// The right to vote is decided solely by what was provided into [`UpdatePotentialVoters()`]. -/// Voting can still fail for [`voter`] that is allowed to vote if, for example, they already voted -/// and policies forbid vote change. -public final function bool AllowedToVote(UserID voter) { +/// Returns true if the user is allowed to vote, false otherwise. +public final function bool IsVotingAllowedFor(UserID voter) { local int i; if (voter == none) {