Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Core: Improve join/leave messages, add "HintGame" tag #2859

Merged
merged 5 commits into from
May 2, 2024
Merged

Core: Improve join/leave messages, add "HintGame" tag #2859

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4c7ba36
Add better "verbs" on joining msg, and improve leaving msgs
EmilyV99 Feb 24, 2024
4854d5c
Add 'HintGame' tag, for projects like BKSudoku/APSudoku/HintMachine
EmilyV99 Feb 24, 2024
c5003bd
data in one place instead of 3
beauxq Mar 2, 2024
e303273
Merge pull request #3 from beauxq/BetterJoinLeaveTags
EmilyV99 Mar 5, 2024
e738459
Clean up 'ignore_game' loop to use any() instead
EmilyV99 Mar 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 31 additions & 4 deletions MultiServer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -804,10 +804,18 @@ async def on_client_joined(ctx: Context, client: Client):
if ctx.client_game_state[client.team, client.slot] == ClientStatus.CLIENT_UNKNOWN:
update_client_status(ctx, client, ClientStatus.CLIENT_CONNECTED)
version_str = '.'.join(str(x) for x in client.version)
verb = "tracking" if "Tracker" in client.tags else "playing"

verbs = {"HintGame": "hinting", "Tracker": "tracking", "TextOnly": "viewing"}
for tag, verb in verbs.items():
if tag in client.tags:
final_verb = verb
break
else:
final_verb = "playing"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be done with a generator expression and next()

I guess it's a stylistic choice as it should perform the same but to me it reads cleaner than a for/else so figured i'd suggest it
 

Suggested change
for tag, verb in verbs.items():
if tag in client.tags:
final_verb = verb
break
else:
final_verb = "playing"
final_verb = next((verb for tag, verb in verbs.items() if tag in client.tags), "playing")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I find this far less readable than the for/else


ctx.broadcast_text_all(
f"{ctx.get_aliased_name(client.team, client.slot)} (Team #{client.team + 1}) "
f"{verb} {ctx.games[client.slot]} has joined. "
f"{final_verb} {ctx.games[client.slot]} has joined. "
f"Client({version_str}), {client.tags}.",
{"type": "Join", "team": client.team, "slot": client.slot, "tags": client.tags})
ctx.notify_client(client, "Now that you are connected, "
Expand All @@ -822,8 +830,20 @@ async def on_client_left(ctx: Context, client: Client):
if len(ctx.clients[client.team][client.slot]) < 1:
update_client_status(ctx, client, ClientStatus.CLIENT_UNKNOWN)
ctx.client_connection_timers[client.team, client.slot] = datetime.datetime.now(datetime.timezone.utc)

version_str = '.'.join(str(x) for x in client.version)

verbs = {"HintGame": "stopped hinting", "Tracker": "stopped tracking", "TextOnly": "stopped viewing"}
for tag, verb in verbs.items():
if tag in client.tags:
final_verb = verb
break
else:
final_verb = "left"

ctx.broadcast_text_all(
"%s (Team #%d) has left the game" % (ctx.get_aliased_name(client.team, client.slot), client.team + 1),
f"{ctx.get_aliased_name(client.team, client.slot)} (Team #{client.team + 1}) has {final_verb} the game. "
f"Client({version_str}), {client.tags}.",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not sure the client version/tags are necessary for leaving, but that might just be me.

But I love the change from "left" to "stopped tracking", will alleviate confusion for sure

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tags is helpful, because for instance my APSudoku hint game has the "APSudoku" tag- so it gives an extra level of clarity to the leave message.

{"type": "Part", "team": client.team, "slot": client.slot})


Expand Down Expand Up @@ -1609,7 +1629,14 @@ async def process_client_cmd(ctx: Context, client: Client, args: dict):
else:
team, slot = ctx.connect_names[args['name']]
game = ctx.games[slot]
ignore_game = ("TextOnly" in args["tags"] or "Tracker" in args["tags"]) and not args.get("game")

ignore_game = False
if not args.get("game"):
for tag in args["tags"]:
if tag in {"HintGame", "TextOnly", "Tracker"}:
ignore_game = True
break

if not ignore_game and args['game'] != game:
errors.add('InvalidGame')
minver = min_client_version if ignore_game else ctx.minimum_client_versions[slot]
Expand Down