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.

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

MultiServer: make !hint without further arguments only reply to the instigating player #2339

Merged
merged 1 commit into from
Mar 3, 2024

Conversation

Berserker66
Copy link
Member

What is this fixing or adding?

Makes it so that !hint only lists the hints to the slot that send the command, not "spamming" anyone else.
I've heard complaints about this feature a few times, so hopefully this improves things.
Also left a note that this was changed to reduce potential player confusion.

How was this tested?

Barely, please test more.

@ThePhar ThePhar added the is: enhancement Issues requesting new features or pull requests implementing new features. label Oct 22, 2023
@@ -649,7 +649,8 @@ def get_aliased_name(self, team: int, slot: int):
else:
return self.player_names[team, slot]

def notify_hints(self, team: int, hints: typing.List[NetUtils.Hint], only_new: bool = False):
def notify_hints(self, team: int, hints: typing.List[NetUtils.Hint], only_new: bool = False,
recipients: typing.Sequence[int] = None):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this is be typing.Optional[typing.Sequence[int]] = None ?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, = None is special cased as already implying Optional

Copy link
Collaborator

@el-u el-u Oct 23, 2023

Choose a reason for hiding this comment

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

This special-casing has been removed and explicit Optional is now recommended instead: python/peps@0630155

Out of scope feature request removed.

@t3hf1gm3nt
Copy link
Collaborator

So yes I know there has been a lot of complaints about other players getting spammed when someone runs the basic !hint command
But I also know there are a fair amount of players who would still want to be able to see hints relevant to them if another player runs the !hint command
Example: One player needs a reminder about something another player hinted for, but said player is still playing their game while the hinting player is sitting in BK
It would be easier/more practical for the BK player to run the hint command so the other player can see what was hinted, rather than having the player who is still playing to stop playing to run it themselves
Note that this is not me disagreeing with this change, but rather pointing out there would be a nonzero amount of players who would still like to have the old functionality in some capacity

@agilbert1412 agilbert1412 added the waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer. label Feb 12, 2024
Copy link
Collaborator

@t3hf1gm3nt t3hf1gm3nt left a comment

Choose a reason for hiding this comment

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

I can't recall if it was implemented at the time of my last comment, but now that we have the hint tab integrated into the client, my previous concerns have been solved by that tab. So I'm good with these changes now.

@Berserker66 Berserker66 merged commit 2c5b2e0 into main Mar 3, 2024
21 checks passed
@Berserker66 Berserker66 deleted the multiserver_personal_hint branch March 3, 2024 05:34
@github-actions github-actions bot removed the waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer. label Mar 3, 2024
EmilyV99 pushed a commit to EmilyV99/Archipelago that referenced this pull request Apr 15, 2024
EmilyV99 pushed a commit to EmilyV99/Archipelago that referenced this pull request Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is: enhancement Issues requesting new features or pull requests implementing new features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants