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

Feature - Kingdom of Miscellania notification message #10933

Merged
merged 1 commit into from Feb 28, 2021
Merged

Feature - Kingdom of Miscellania notification message #10933

merged 1 commit into from Feb 28, 2021

Conversation

BrandtHill
Copy link
Contributor

@BrandtHill BrandtHill commented Mar 3, 2020 •
edited

I dreamt of an additional feature for the KingdomPlugin several months and finally decided to implement it. Upon login, if the user is managing the kingdom, their coffer and favor will be sent as a chat notification. The values are calculated using the last known coffer and favor values the same way the server calculates them.

image

Let me know if I should create a corresponding issue for this feature and amend this PR to close it.
closes #10939

Brandt

@BrandtHill BrandtHill mentioned this pull request Mar 3, 2020
Closed
raiyni
raiyni requested changes Mar 3, 2020
View reviewed changes
Copy link
Member

@raiyni raiyni left a comment

Choose a reason for hiding this comment

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

You can use this page to get our style guide and import our formatter

https://github.com/runelite/runelite/wiki/Code-Conventions

...ite-client/src/main/java/net/runelite/client/plugins/kingdomofmiscellania/KingdomConfig.java Outdated Show resolved Hide resolved
...ite-client/src/main/java/net/runelite/client/plugins/kingdomofmiscellania/KingdomConfig.java Outdated Show resolved Hide resolved
...ite-client/src/main/java/net/runelite/client/plugins/kingdomofmiscellania/KingdomConfig.java Outdated Show resolved Hide resolved
...ite-client/src/main/java/net/runelite/client/plugins/kingdomofmiscellania/KingdomConfig.java Outdated Show resolved Hide resolved
...ite-client/src/main/java/net/runelite/client/plugins/kingdomofmiscellania/KingdomConfig.java Outdated Show resolved Hide resolved
...ite-client/src/main/java/net/runelite/client/plugins/kingdomofmiscellania/KingdomPlugin.java Outdated Show resolved Hide resolved
...ite-client/src/main/java/net/runelite/client/plugins/kingdomofmiscellania/KingdomPlugin.java Outdated Show resolved Hide resolved
Nightfirecat
Nightfirecat approved these changes Mar 8, 2020
View reviewed changes
Copy link
Member

@Nightfirecat Nightfirecat left a comment

Choose a reason for hiding this comment

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

LGTM

@BrandtHill
Copy link
Contributor Author

Let me know if there's anything that needs to be changed.

@Nightfirecat Nightfirecat requested a review from Adam- March 25, 2020 18:59
@BrandtHill
Copy link
Contributor Author

Happy to make any requested changes for approval.

@BrandtHill BrandtHill mentioned this pull request Aug 4, 2020
Closed
@BrandtHill
Copy link
Contributor Author

Let me know if anything needs to be changed.

Nightfirecat
Nightfirecat reviewed Jan 8, 2021
View reviewed changes
Copy link
Member

@Nightfirecat Nightfirecat left a comment

Choose a reason for hiding this comment

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

Mostly small fixups. This PR generally looks good

...ite-client/src/main/java/net/runelite/client/plugins/kingdomofmiscellania/KingdomPlugin.java Outdated Show resolved Hide resolved
...ite-client/src/main/java/net/runelite/client/plugins/kingdomofmiscellania/KingdomConfig.java Outdated Show resolved Hide resolved
...ite-client/src/main/java/net/runelite/client/plugins/kingdomofmiscellania/KingdomConfig.java Outdated Show resolved Hide resolved
...ite-client/src/main/java/net/runelite/client/plugins/kingdomofmiscellania/KingdomPlugin.java Outdated Show resolved Hide resolved
...ite-client/src/main/java/net/runelite/client/plugins/kingdomofmiscellania/KingdomPlugin.java Outdated Show resolved Hide resolved
...ite-client/src/main/java/net/runelite/client/plugins/kingdomofmiscellania/KingdomPlugin.java Outdated Show resolved Hide resolved
...ite-client/src/main/java/net/runelite/client/plugins/kingdomofmiscellania/KingdomPlugin.java Outdated Show resolved Hide resolved
@BrandtHill
Copy link
Contributor Author

Thanks for re-reviewing. Suggested changes have been committed.

Nightfirecat
Nightfirecat approved these changes Jan 12, 2021
View reviewed changes
Copy link
Member

@Nightfirecat Nightfirecat left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for keeping up with this PR all this time!

@BrandtHill
Copy link
Contributor Author

LGTM. Thanks for keeping up with this PR all this time!

I never lost hope!

@BrandtHill BrandtHill force-pushed the miscellania-notification-message branch from 1947732 to d86ee19 Compare February 16, 2021 01:12
@BrandtHill
Copy link
Contributor Author

BrandtHill commented Feb 16, 2021 •
edited

Regrettably, I no longer have a membership. Would someone be so kind as to test this? Simply visit Miscellania, enable the notification message, leave the area, and relog.
You can edit your profile file kingdomofmiscellania.{profile}.lastChanged and set it back a few days, then log in and verify the estimated values are less than their current values.

@Nightfirecat
Copy link
Member

The changes look fine to me. I'll put some money in miscellania and play with this feature enabled for a few days and see how it looks.

@Nightfirecat
Copy link
Member

This doesn't seem to be working currently. I turned on notifications with a coffer threshold of 0 and a favor threshold of 100, but got no notification a day and a half later after depositing some money into my kingdom coffers. I'll look at this more later to see if I can't figure out why it's not working.

@BrandtHill
Copy link
Contributor Author

Thanks for testing. I'll get a membership and test this out.

@BrandtHill
Copy link
Contributor Author

I've fixed bugs relating to the new config functions.

I've simulated the passage of time by calling the following function with different values and logging on and verifying the message is sent with approximations.

setLastChanged(Instant.ofEpochSecond(1613522615));
setCoffer(1_000_000);
setFavor(127);

@Adam- Adam- force-pushed the miscellania-notification-message branch from 55da30e to e23b9d2 Compare February 27, 2021 22:47
@BrandtHill BrandtHill force-pushed the miscellania-notification-message branch from e23b9d2 to 9d44ebf Compare February 28, 2021 01:18
@BrandtHill
Copy link
Contributor Author

I renamed everything from favor to approval and changed the guard to ||

@Adam-
kingdom: add notification message for favor and coffer value
d0ec76e 
This allows setting a configurable threshold where you will receive a
message informing you your favor and/or coffer is low on login

Co-authored-by: Adam <Adam@sigterm.info>
@Adam- Adam- force-pushed the miscellania-notification-message branch from 9d44ebf to d0ec76e Compare February 28, 2021 02:12
@Adam- Adam- merged commit 2d5d490 into runelite:master Feb 28, 2021
@BrandtHill
Copy link
Contributor Author

Just here to say happy birthday to this PR. You would have been one year old today. Merged before your time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Trevor159 Trevor159 Awaiting requested review from Trevor159

@raiyni raiyni Awaiting requested review from raiyni

@Nightfirecat Nightfirecat Awaiting requested review from Nightfirecat

@Adam- Adam- Awaiting requested review from Adam-

Assignees

@Adam- Adam-

Labels
feature-request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feature - Kingdom of Miscellania notification message
6 participants
@BrandtHill @Nightfirecat @Adam- @Trevor159 @raiyni @deathbeam