Skip to content

Conversation

@Minc3
Copy link

@Minc3 Minc3 commented May 10, 2022

No description provided.

@Minc3 Minc3 changed the title Added waiting for staff option and fixed sam remove slays option Added waiting for staff and fixed sam remove slays May 10, 2022
@Minc3
Copy link
Author

Minc3 commented May 10, 2022

Now it's easy to tell when a report is ready for staff.
Screenshot from 2022-05-10 22-47-37

@BadgerCode
Copy link
Owner

Awesome thank you!
I'll try take a look over this later today

@Minc3
Copy link
Author

Minc3 commented May 10, 2022

Awesome,
The empty string I did for SAM is required because SAM expects there to be a reason for why you are removing the slay so I just put a space there so the command would work.

@BadgerCode
Copy link
Owner

Sorry I will need to test this tomorrow/later this week. I didn't have time today.
Once I've tested it, I will release your changes!

@BadgerCode
Copy link
Owner

Hm. I gave it a test, but for some reason the menu still showed the report as "Waiting"
This might be an issue with how I was testing it (with bots, local server/peer-to-peer server via the main menu)

From what I could tell, the status RDM_MANAGER_READYFORSTAFF was making it from the server to client, but for some reason the menu still shows the waiting status.

I shall have to give it another test

@Minc3
Copy link
Author

Minc3 commented May 12, 2022

The status is changed to waiting for staff when the victim has decided to keep the report and not forgive them.

So you will need to report a player that can respond to your report and when you see their response you are giving two options keep or forgive.

If you select keep then the status of the report is set to waiting for staff.

I tested with players not bots and a dedicated server.

@BadgerCode
Copy link
Owner

Hey!

Sorry for the delay on this. Busy few days for me.
I will give this another test sometime this week!

Copy link

@callumok2004 callumok2004 left a comment

Choose a reason for hiding this comment

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

When a report status is set to in progress or finished, there is a check to ensure the previous status was "waiting", this does not check if the previous status could have also been "waiting for staff" causing the pending reports to be invalid.

@BadgerCode
Copy link
Owner

Hey.
Sorry for the delay on this. I've just not had the time for it 🙁

Thanks for taking a look at this @callumok2004
That does sound like a problem that might need fixing @Minc3

@Minc3
Copy link
Author

Minc3 commented Jul 22, 2022

I have found a few more SAM related issues that I would like to fix in this PR before merge.

@callumok2004 I believe that should fix the issue.
I'll review all my changes just to make sure I haven't missed anything.

callumok2004
callumok2004 previously approved these changes Jul 22, 2022
@Minc3
Copy link
Author

Minc3 commented Aug 19, 2022

This weekend I'll have time to fix all the remaining SAM issues.

@Minc3
Copy link
Author

Minc3 commented Aug 19, 2022

I am testing all of the changes on my TTT server.

@Minc3
Copy link
Author

Minc3 commented Aug 19, 2022

Ready to be merged

@Minc3
Copy link
Author

Minc3 commented Aug 19, 2022

Fixed #51

@Minc3 Minc3 changed the title Added waiting for staff and fixed sam remove slays Added waiting for staff and fixed sam Aug 19, 2022
@BadgerCode BadgerCode changed the base branch from master to next-version January 29, 2023 15:20
@BadgerCode BadgerCode dismissed callumok2004’s stale review January 29, 2023 15:20

The base branch was changed.

Copy link
Owner

@BadgerCode BadgerCode left a comment

Choose a reason for hiding this comment

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

Overall it looks fine, but I think this could do with some comprehensive testing as introducing a new status for reports is quite a significant change.

@Satton2
Copy link

Satton2 commented Mar 25, 2023

@Minc3
https://youtu.be/SGfP8StcSAI
Hello there, found an interesting issue/bug with this status. You see, both the "Waiting" status and the "Waiting for staff" add +1 to the "Pending reports" counter, resulting in never ending pending reports.

EDIT: I believe that SAM related changes should be made in seperate PR to merge them right now, while new status will be waiting for it's time.

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.

4 participants