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

Add SanctionReason #595

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

NikoCat233
Copy link
Contributor

image

bandicam.2024-05-05.22-29-24-670.mp4

@NikoCat233
Copy link
Contributor Author

To use this, DisconnectReason should be DisconnectReason.Sanctions
and you must assign a EndsAt

Copy link
Member

Choose a reason for hiding this comment

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

Did you manually create this file? It should be automatically generated using Dumpostor: https://github.com/Impostor/Dumpostor/blob/master/Dumpostor/DumpostorPlugin.cs#L90

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hm what is this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh i see, i will gen it

@NikoCat233
Copy link
Contributor Author

NikoCat233 commented May 5, 2024

Now its dumped with Dumpostor
Litterly Same with what I manully entered lol

@AeonLucid
Copy link
Collaborator

Litterly Same with what I manully entered lol

Makes maintaining and updating it easier in the future

Copy link
Member

@miniduikboot miniduikboot left a comment

Choose a reason for hiding this comment

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

This PR doesn't work correctly: the message that is sent when joining a non-existing game changed to something more confusing.

Reproduction steps:

  1. Start Impostor with a build from your branch.
  2. Join the non-existent game "AAAA"
  3. Wait for the error message.

Before: Could not find the game you're looking for.
After: There was a problem finding a game code.\n Not found. Code: 404

I don't believe this change is intended by you, could you look into why this message changed and make sure the old message is shown again?

@js6pak
Copy link
Member

js6pak commented May 8, 2024

Before: Could not find the game you're looking for. After: There was a problem finding a game code.\n Not found. Code: 404

This was caused by the game client not accepting null EndsAt values and crashing

I've pushed a fix and this is how the game displays errors now:

new MatchmakerError(DisconnectReason.GameNotFound)
image

new MatchmakerError(SanctionReason.CheatingHacking, DateTimeOffset.UtcNow)
image

new MatchmakerError(SanctionReason.CheatingHacking, DateTimeOffset.MaxValue)
image

Copy link
Member

@js6pak js6pak left a comment

Choose a reason for hiding this comment

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

All of this is currently internal, so what's the point of this PR? Do we want to open this up for plugins?

src/Impostor.Api.Innersloth.Generator/SourceGenerator.cs Outdated Show resolved Hide resolved
@@ -1,5 +1,5 @@
{
"dumpostorVersion": "1.0.0",
"dumpostorVersion": "1.0.0-dev",
Copy link
Member

Choose a reason for hiding this comment

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

This should be regenerated with release dumpostor before merging

NikoCat233 and others added 2 commits May 9, 2024 16:46
@NikoCat233
Copy link
Contributor Author

All of this is currently internal, so what's the point of this PR? Do we want to open this up for plugins?

Probably, im using it on my own server

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants