Skip to content

Commit

Permalink
Merged PR 296: v1.25.0 - Fix prompt bug, allow inactive subs (#262)
Browse files Browse the repository at this point in the history 
- Fix regression where the new game prompt did nothing on Yes and No
- Allow inactive players to join from the bench without having to sub in
- Various security updates to dependencies
- Bump version to 1.25.0
  • Loading branch information
@alopezlago
alopezlago authored Oct 27, 2023
1 parent dac2860 commit 4bb8aa1
Show file tree
Hide file tree
Showing 23 changed files with 455 additions and 71 deletions.
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "modaq",
"version": "1.24.2",
"version": "1.25.0",
"description": "Quiz Bowl Reader using TypeScript, React, and MobX",
"repository": {
"type": "git",
Expand Down
85 changes: 63 additions & 22 deletions src/components/GameBar.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -427,21 +427,19 @@ function getPlayerManagementSubMenuItems(
for (const teamName of teamNames) {
const players: Player[] = game.getPlayers(teamName);
const activePlayers: Set<Player> = game.getActivePlayers(teamName, uiState.cycleIndex);

// Potential issue: if we have an "add player" sub, they shouldn't appear before they were added?
const subs: Player[] = players.filter((player) => !activePlayers.has(player));

const activePlayerMenuItems: ICommandBarItemProps[] = [];
for (const activePlayer of activePlayers) {
const subMenuItems: ICommandBarItemProps[] = subs.map((player) => {
for (const player of players) {
const subMenuItems: ICommandBarItemProps[] = subs.map((p) => {
const subItemData: ISubMenuItemData = {
appState,
activePlayer,
player,
activePlayer: player,
player: p,
};
return {
key: `sub_${teamName}_${player.name}`,
text: player.name,
key: `sub_${teamName}_${p.name}`,
text: p.name,
data: subItemData,
onClick: onSwapPlayerClick,
};
Expand All @@ -457,30 +455,49 @@ function getPlayerManagementSubMenuItems(
},
};

const removeItemData: ISubMenuItemData = {
const changeActivityItemData: ISubMenuItemData = {
appState,
activePlayer,
};
const removeItem: ICommandBarItemProps = {
key: `remove_${teamName}_${activePlayer.name}`,
text: "Remove",
data: removeItemData,
onClick: onRemovePlayerClick,
// TODO: should this be styled in a different color?
activePlayer: player,
};

// For active players, remove. For inactive players, join
const isActivePlayer: boolean = activePlayers.has(player);
let changeActivityItem: ICommandBarItemProps;
if (isActivePlayer) {
changeActivityItem = {
key: `remove_${teamName}_${player.name}`,
text: "Remove",
data: changeActivityItemData,
onClick: onRemovePlayerClick,
// TODO: should this be styled in a different color?
};
} else {
changeActivityItem = {
key: `add_${teamName}_${player.name}`,
text: "Add",
data: changeActivityItemData,
onClick: onAddInactivePlayerClick,
// TODO: should this be styled in a different color?
};
}

const renameItem: ICommandBarItemProps = {
key: `rename_${teamName}_${activePlayer.name}`,
key: `rename_${teamName}_${player.name}`,
text: "Rename",
data: removeItemData,
data: changeActivityItemData,
onClick: onRenamePlayerClick,
// TODO: should this be styled in a different color?
};

const items: ICommandBarItemProps[] = isActivePlayer
? [subMenuSectionItem, changeActivityItem, renameItem]
: [changeActivityItem, renameItem];

activePlayerMenuItems.push({
key: `active_${teamName}_${activePlayer.name}`,
text: activePlayer.name,
key: `active_${teamName}_${player.name}`,
text: player.name,
subMenuProps: {
items: [subMenuSectionItem, removeItem, renameItem],
items,
},
});
}
Expand Down Expand Up @@ -508,10 +525,20 @@ function getPlayerManagementSubMenuItems(
// We should disable this if there are no subs available
};

// Couple possible approaches
// - Submenu for Add Player, where it's Add New Player and then all the subs
// - Add all players to the sub/remove submenu, but the subs action item is just "join"/"rename"
// - You can get some weird behavior with joins/subs in the same event. May need to disable it if they have an
// existing action (sub vs join)
// - Should there be a color code for active players?

const addPlayerItem: ICommandBarItemProps = {
key: "addNewPlayer",
text: "Add player...",
onClick: addPlayerHandler,
// subMenuProps: {
// items: addPlayerMenus,
// },
disabled: appState.game.cycles.length === 0,
};

Expand Down Expand Up @@ -634,6 +661,20 @@ function getProtestSubMenuItems(
};
}

function onAddInactivePlayerClick(
ev?: React.MouseEvent<HTMLElement> | React.KeyboardEvent<HTMLElement>,
item?: IContextualMenuItem
): void {
if (item == undefined) {
return;
} else if (!isSubMenuItemData(item.data)) {
return;
}

const appState: AppState = item.data.appState;
appState.game.addInactivePlayer(item.data.activePlayer, appState.uiState.cycleIndex);
}

function onProtestTossupClick(
ev?: React.MouseEvent<HTMLElement> | React.KeyboardEvent<HTMLElement>,
item?: IContextualMenuItem
Expand Down
2 changes: 1 addition & 1 deletion src/components/dialogs/AddPlayerDialogController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export function addPlayer(): void {
throw new Error("Tried adding a player with no new player");
}

game.addPlayer(newPlayer);
game.addNewPlayer(newPlayer);

// TODO: Only do this if the number of active players is less than the maximum number of active players
game.cycles[uiState.cycleIndex].addPlayerJoins(newPlayer);
Expand Down
4 changes: 2 additions & 2 deletions src/components/dialogs/ImportGameDialog.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,8 @@ function onSubmit(appState: AppState): void {

// We need to set the game's packet, players, etc. to the values in the uiState
game.clear();
game.addPlayers(firstTeamPlayers.filter((player) => player.name !== ""));
game.addPlayers(secondTeamPlayers.filter((player) => player.name !== ""));
game.addNewPlayers(firstTeamPlayers.filter((player) => player.name !== ""));
game.addNewPlayers(secondTeamPlayers.filter((player) => player.name !== ""));
game.loadPacket(pendingNewGame.packet);
game.setCycles(pendingNewGame.manual.cycles ?? []);

Expand Down
4 changes: 2 additions & 2 deletions src/components/dialogs/NewGameDialog.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -630,8 +630,8 @@ function onSubmit(appState: AppState): void {
// We need to set the game's packet, players, etc. to the values in the uiState
const game: GameState = appState.game;
game.clear();
game.addPlayers(firstTeamPlayers.filter((player) => player.name !== ""));
game.addPlayers(secondTeamPlayers.filter((player) => player.name !== ""));
game.addNewPlayers(firstTeamPlayers.filter((player) => player.name !== ""));
game.addNewPlayers(secondTeamPlayers.filter((player) => player.name !== ""));
game.loadPacket(pendingNewGame.packet);

game.setGameFormat(pendingNewGame.gameFormat);
Expand Down
26 changes: 19 additions & 7 deletions src/state/DialogState.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,16 @@ export class DialogState {

public hideAddQuestionsDialog(): void {
this.addQuestions = undefined;
this.hideModalDialog();
if (this.visibleDialog === ModalVisibilityStatus.AddQuestions) {
this.hideModalDialog();
}
}

public hideCustomizeGameFormatDialog(): void {
this.customizeGameFormat = undefined;
this.hideModalDialog();
if (this.visibleDialog === ModalVisibilityStatus.CustomizeGameFormat) {
this.hideModalDialog();
}
}

public hideModalDialog(): void {
Expand All @@ -60,22 +64,30 @@ export class DialogState {

public hideFontDialog(): void {
this.fontDialog = undefined;
this.hideModalDialog();
if (this.visibleDialog === ModalVisibilityStatus.Font) {
this.hideModalDialog();
}
}

public hideMessageDialog(): void {
this.messageDialog = undefined;
this.hideModalDialog();
if (this.visibleDialog === ModalVisibilityStatus.Message) {
this.hideModalDialog();
}
}

public hideRenamePlayerDialog(): void {
this.renamePlayerDialog = undefined;
this.hideModalDialog();
if (this.visibleDialog === ModalVisibilityStatus.RenamePlayer) {
this.hideModalDialog();
}
}

public hideReorderPlayersDialog(): void {
this.reorderPlayersDialog = undefined;
this.hideModalDialog();
if (this.visibleDialog === ModalVisibilityStatus.ReorderPlayers) {
this.hideModalDialog();
}
}

public showAddQuestionsDialog(): void {
Expand Down Expand Up @@ -155,7 +167,7 @@ export class DialogState {
message,
type: MessageDialogType.YesNocCancel,
onOK: onYes,
onNo,
onNo: onNo,
};
this.visibleDialog = ModalVisibilityStatus.Message;
}
Expand Down
104 changes: 100 additions & 4 deletions src/state/GameState.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { computed, observable, action, makeObservable, when } from "mobx";
import { format } from "mobx-sync";

import * as GameFormats from "./GameFormats";
import * as PlayerUtils from "./PlayerUtils";
import { PacketState, Bonus, Tossup } from "./PacketState";
import { IPlayer, Player } from "./TeamState";
import { Cycle, ICycle } from "./Cycle";
Expand Down Expand Up @@ -53,8 +54,9 @@ export class GameState {
finalScore: computed,
playableCycles: computed,
scores: computed,
addPlayer: action,
addPlayers: action,
addInactivePlayer: action,
addNewPlayer: action,
addNewPlayers: action,
clear: action,
loadPacket: action,
setCycles: action,
Expand Down Expand Up @@ -238,11 +240,105 @@ export class GameState {
];
}

public addPlayer(player: Player): void {
public addInactivePlayer(player: Player, cycleIndex: number): void {
for (let i = cycleIndex; i >= 0; i--) {
const cycle: Cycle = this.cycles[i];

if (
cycle.playerJoins &&
cycle.playerJoins.some((joinEvent) => PlayerUtils.playersEqual(player, joinEvent.inPlayer))
) {
// We have a current or earlier join event with no leave events. Adding a player now is a no-op.
return;
}

if (cycle.subs) {
if (cycle.subs.some((subEvent) => PlayerUtils.playersEqual(player, subEvent.inPlayer))) {
// We have a current or earlier join event with no leave events. Adding a player now is a no-op.
return;
} else if (cycle.subs.some((subEvent) => PlayerUtils.playersEqual(player, subEvent.outPlayer))) {
// Need to test subbing someone out and also having them join. Very strange set of events.
break;
}
}

if (
cycle.playerLeaves &&
cycle.playerLeaves.some((leaveEvent) => PlayerUtils.playersEqual(player, leaveEvent.outPlayer))
) {
// If it's the same cycle as the current one, remove the playerLeaves event, since this has priority
// as we're explicitly adding it afterwards
if (i == cycleIndex) {
const leaveEvent: IPlayerLeavesEvent | undefined = cycle.playerLeaves.find((leaveEvent) =>
PlayerUtils.playersEqual(player, leaveEvent.outPlayer)
);

if (leaveEvent != undefined) {
cycle.removePlayerLeaves(leaveEvent);
}
}

break;
}
}

for (let i = cycleIndex + 1; i < this.cycles.length; i++) {
const cycle: Cycle = this.cycles[i];

if (
cycle.playerLeaves &&
cycle.playerLeaves.some((leaveEvent) => PlayerUtils.playersEqual(player, leaveEvent.outPlayer))
) {
break;
} else if (
cycle.subs &&
cycle.subs.some((subEvent) => PlayerUtils.playersEqual(player, subEvent.outPlayer))
) {
break;
}

if (cycle.subs) {
if (cycle.subs.some((subEvent) => PlayerUtils.playersEqual(player, subEvent.inPlayer))) {
// We're adding the player earlier, and there's no leave event. Remove that event and then add the new
// one
const subEvent: IPlayerJoinsEvent | undefined = cycle.subs.find(
(joinEvent) => !PlayerUtils.playersEqual(player, joinEvent.inPlayer)
);
if (subEvent) {
cycle.removePlayerJoins(subEvent);
}

break;
} else if (cycle.subs.some((subEvent) => PlayerUtils.playersEqual(player, subEvent.outPlayer))) {
break;
}
}

if (
cycle.playerJoins &&
cycle.playerJoins.some((joinEvent) => PlayerUtils.playersEqual(player, joinEvent.inPlayer))
) {
// We're adding the player earlier, and there's no leave event. Remove that event and then add the new
// one
const joinEvent: IPlayerJoinsEvent | undefined = cycle.playerJoins.find((joinEvent) =>
PlayerUtils.playersEqual(player, joinEvent.inPlayer)
);
if (joinEvent) {
cycle.removePlayerJoins(joinEvent);
}

break;
}
}

this.cycles[cycleIndex].addPlayerJoins(player);
}

public addNewPlayer(player: Player): void {
this.players.push(player);
}

public addPlayers(players: Player[]): void {
public addNewPlayers(players: Player[]): void {
this.players.push(...players);
}

Expand Down
Loading

0 comments on commit 4bb8aa1

Please sign in to comment.