Custom research unlock #3942

StarWishsama · 2023-08-09T08:10:38Z

Description

Experience isn't only way to retrieve knowledge, money also can.

Added SlimefunGuideUnlockMode and SlimefunGuideUnlockProvider to support more ways to unlock research.
Support use player's server currency to unlock research with Vault API.

Proposed changes

Added SlimefunGuideUnlockMode
Added SlimefunGuideUnlockProvider
Added VaultIntegration for integration of unlock research.

Related Issues (if applicable)

Checklist

I have fully tested the proposed changes and promise that they will not break everything into chaos.
I have also tested the proposed changes in combination with various popular addons and can confirm my changes do not break them.
I have made sure that the proposed changes do not break compatibility across the supported Minecraft versions (1.16.* - 1.20.*).
I followed the existing code standards and didn't mess up the formatting.
I did my best to add documentation to any public classes or methods I added.
I have added Nonnull and Nullable annotations to my methods to indicate their behaviour for null values
I added sufficient Unit Tests to cover my code.

github-actions · 2023-08-09T08:11:00Z

Your Pull Request was automatically labelled as: "🎈 Feature"
Thank you for contributing to this project! ❤️

Sefiraat · 2023-08-09T08:18:46Z

Not sure on others' feelings on this, but I would rather var wasn't used over directly declaring the datatype...

J3fftw1

quick review
and i agree with Sefi please dont use var

src/main/java/io/github/thebusybiscuit/slimefun4/integrations/IntegrationsManager.java

src/main/java/io/github/thebusybiscuit/slimefun4/integrations/VaultIntegration.java

src/main/java/io/github/thebusybiscuit/slimefun4/core/guide/SlimefunGuideUnlockMode.java

Sefiraat · 2023-08-09T08:36:17Z

Side comment, freaking love this. :)

StarWishsama · 2023-08-09T08:40:58Z

Not sure why everyone dislike var even we're already in Java 16... But i will change it anyway.

Sefiraat · 2023-08-09T08:43:01Z

Not sure why everyone dislike var even we're already in Java 16... But i will change it anyway.

My personal dislike is that Slimefun attracts new developers often, and var means a developer reading the code has to know the context of the incoming object. This isn't the worst thing it's just a barrier that I don't think is needed. Specifically declaring the type means no context is required by those reading the code later. Its a small thing, but given var doesn't really provide alot of benefit (imo) I think it's worth the slightly uglier code :)

J3fftw1 · 2023-08-09T08:49:08Z

I love readable code. Im also all for doing full variable names instead of P. since we are on java 16 we can but dont have to use var but i would opt out of using it because of readability

github-actions · 2023-08-09T09:31:38Z

Slimefun preview build

A Slimefun preview build is available for testing!
Commit: b6c16892

https://preview-builds.walshy.dev/download/Slimefun/3942/b6c16892

Note: This is not a supported build and is only here for the purposes of testing.
Do not run this on a live server and do not report bugs anywhere but this PR!

src/main/java/io/github/thebusybiscuit/slimefun4/api/researches/Research.java

src/main/java/io/github/thebusybiscuit/slimefun4/core/SlimefunRegistry.java

...ava/io/github/thebusybiscuit/slimefun4/core/guide/unlockprovider/CurrencyUnlockProvider.java

...a/io/github/thebusybiscuit/slimefun4/core/guide/unlockprovider/ExperienceUnlockProvider.java

iTwins

Just looked at style

src/main/java/io/github/thebusybiscuit/slimefun4/integrations/VaultIntegration.java

src/main/java/io/github/thebusybiscuit/slimefun4/core/guide/SlimefunGuideUnlockMode.java

src/main/java/io/github/thebusybiscuit/slimefun4/api/researches/Research.java

src/main/java/io/github/thebusybiscuit/slimefun4/api/guide/SlimefunGuideUnlockProvider.java

...ava/io/github/thebusybiscuit/slimefun4/core/guide/unlockprovider/CurrencyUnlockProvider.java

src/main/java/io/github/thebusybiscuit/slimefun4/core/guide/SlimefunGuideUnlockMode.java

src/main/java/io/github/thebusybiscuit/slimefun4/integrations/VaultIntegration.java

…VaultIntegration.java Co-authored-by: Jeroen <[email protected]>

…imefunGuideUnlockMode.java Co-authored-by: Jeroen <[email protected]>

Co-authored-by: Jeroen <[email protected]>

src/main/java/io/github/thebusybiscuit/slimefun4/api/events/SlimefunGuideOpenEvent.java

src/main/java/io/github/thebusybiscuit/slimefun4/core/guide/SlimefunGuideUnlockMode.java

src/main/java/io/github/thebusybiscuit/slimefun4/core/handlers/RainbowTickHandler.java

src/main/java/io/github/thebusybiscuit/slimefun4/integrations/VaultIntegration.java

TheBusyBiscuit

I have to say this PR has a rather large footprint...

while more configuration is good, it needs to be maintainable, the research system itself is too rusty and old in my opinion to be adding onto it such a large feature.
But idk if a research rewrite is gonna be coming anytime soon, so not sure about this really.

StarWishsama · 2023-08-14T15:43:31Z

I have to say this PR has a rather large footprint...

while more configuration is good, it needs to be maintainable, the research system itself is too rusty and old in my opinion to be adding onto it such a large feature.
But idk if a research rewrite is gonna be coming anytime soon, so not sure about this really.

3/4 of those are line separator change, nice work github.

…research

…research # Conflicts: # src/main/java/io/github/thebusybiscuit/slimefun4/api/events/SlimefunGuideOpenEvent.java

src/main/java/io/github/thebusybiscuit/slimefun4/core/guide/SlimefunGuideUnlockMode.java

Boomer-1 · 2023-11-23T01:18:32Z

can confirm it works in survival mode. if player is in creative mode currency is not deducted. Additionally I would recommend a comment be added to the config file to display the options that the server owner can type in that line, if it's more than currency or experience. it wasn't intuitive. I had to go into the code to find that. I also only tested currency. didn't test tokens or other options

JustAHuman-xD · 2023-11-23T01:25:59Z

src/main/resources/languages/en/messages.yml

-  not-enough-xp: '&4You do not have enough XP to unlock this'
+  not-meet-requirement: '&4You do not meet the requirement to unlock this'


should probably call it something like does-not-meet-requirement or something like that

maybe requirement-unfulfilled? or unfulfilled-requirement

...a/io/github/thebusybiscuit/slimefun4/core/guide/unlockprovider/ExperienceUnlockProvider.java

...ava/io/github/thebusybiscuit/slimefun4/core/guide/unlockprovider/CurrencyUnlockProvider.java

JustAHuman-xD · 2023-11-23T01:29:34Z

src/main/java/io/github/thebusybiscuit/slimefun4/core/SlimefunRegistry.java

@@ -72,6 +73,7 @@ public final class SlimefunRegistry {
    private boolean disableLearningAnimation;
    private boolean logDuplicateBlockEntries;
    private boolean talismanActionBarMessages;
+    private SlimefunGuideUnlockMode guideUnlockMode;


I think a setter method for this would be nice

Any demand for setter method? The original goal is implement a research scoped custom unlock provider for addon dev and a global bulit-in unlock provider.

src/main/java/io/github/thebusybiscuit/slimefun4/api/researches/Research.java

…research

J3fftw1 · 2023-12-31T09:50:06Z

I would still love this change in.
People have requested this a bunch in the past.
but it has merge conflicts
not gonna marking it as stale since the merge conflicts got added 4 days ago

StarWishsama · 2023-12-31T10:46:36Z

I would still love this change in.
People have requested this a bunch in the past.
but it has merge conflicts
not gonna marking it as stale since the merge conflicts got added 4 days ago

Gonna resolve this at first day in 2024 😄

…research # Conflicts: # pom.xml

StarWishsama added 5 commits August 8, 2023 23:45

feat(research): economy unlock research

650f47a

chore: typo fix

10be7ce

chore: annotation fix

4106eda

fix(vault): validate vault status on unlocking

c3b1a91

chore: cleanup import

c7f4daa

StarWishsama requested review from a team as code owners August 9, 2023 08:10

github-actions bot added the 🎈 Feature This Pull Request adds a new feature. label Aug 9, 2023

J3fftw1 added the 🎯 Needs testing This Issue needs to be tested by our team to see if it can be reproduced. label Aug 9, 2023

J3fftw1 suggested changes Aug 9, 2023

View reviewed changes

Sefiraat reviewed Aug 9, 2023

View reviewed changes

src/main/java/io/github/thebusybiscuit/slimefun4/core/guide/SlimefunGuideUnlockMode.java Outdated Show resolved Hide resolved

feat(guide): support custom unlock provider

0d3d3a4

StarWishsama added 3 commits August 9, 2023 17:32

chore: remove var usage

e4c3b50

chore: unlock unit fix

694080f

chore: relocate SlimefunGuideUnlockProvider

a676060

J3fftw1 suggested changes Aug 9, 2023

View reviewed changes

style: apply suggestion

117a232

StarWishsama changed the title ~~Support unlock research by using currency~~ Support custom method to unlock research Aug 9, 2023

iTwins suggested changes Aug 9, 2023

View reviewed changes

StarWishsama and others added 4 commits August 10, 2023 12:00

Update src/main/java/io/github/thebusybiscuit/slimefun4/integrations/…

18f8141

…VaultIntegration.java Co-authored-by: Jeroen <[email protected]>

Update src/main/java/io/github/thebusybiscuit/slimefun4/core/guide/Sl…

59d8ed1

…imefunGuideUnlockMode.java Co-authored-by: Jeroen <[email protected]>

style: apply suggestion

fb8f6d7

Co-authored-by: Jeroen <[email protected]>

fix: missing import

f241c80

J3fftw1 suggested changes Aug 14, 2023

View reviewed changes

TheBusyBiscuit reviewed Aug 14, 2023

View reviewed changes

Phoenix-Starlight mentioned this pull request Aug 14, 2023

Enforce LF endings automatically #3946

Merged

7 tasks

StarWishsama added 2 commits August 16, 2023 13:58

chore: more typo fix

328f109

Merge remote-tracking branch 'upstream/master' into feature/currency-…

5808e9a

…research

TheBusyBot added the ⚡ Merge Conflicts This Pull Request has merged conflicts which need to be resolved! label Aug 30, 2023

Merge remote-tracking branch 'upstream/master' into feature/currency-…

ee2cbf9

…research # Conflicts: # src/main/java/io/github/thebusybiscuit/slimefun4/api/events/SlimefunGuideOpenEvent.java

StarWishsama changed the title ~~Support custom method to unlock research~~ Custom research unlock Sep 10, 2023

StarWishsama added 2 commits September 10, 2023 16:09

chore: use Preconditions

ce2c026

chore: use Preconditions

a2ed3cf

StarWishsama requested a review from J3fftw1 September 10, 2023 08:14

chore: fix condition check

38d04df

J3fftw1 suggested changes Sep 10, 2023

View reviewed changes

J3fftw1 removed the ⚡ Merge Conflicts This Pull Request has merged conflicts which need to be resolved! label Sep 10, 2023

chore: typo fix

9644364

StarWishsama requested a review from J3fftw1 September 10, 2023 12:49

ybw0014 mentioned this pull request Sep 21, 2023

Added the ability to change the text: Click to Unlock. As well as level and cost. #3977

Closed

4 tasks

ybw0014 mentioned this pull request Oct 20, 2023

Research unlock with vault economy provider #4002

Open

7 tasks

JustAHuman-xD reviewed Nov 23, 2023

View reviewed changes

StarWishsama added 2 commits November 25, 2023 11:09

Merge remote-tracking branch 'upstream/master' into feature/currency-…

9235128

…research

chore(provider): implemented suggestion

529138b

TheBusyBot added the ⚡ Merge Conflicts This Pull Request has merged conflicts which need to be resolved! label Dec 27, 2023

Merge remote-tracking branch 'upstream/master' into feature/currency-…

ae752b3

…research # Conflicts: # pom.xml

StarWishsama force-pushed the feature/currency-research branch from cc30ef1 to ae752b3 Compare January 5, 2024 03:16

TheBusyBot removed the ⚡ Merge Conflicts This Pull Request has merged conflicts which need to be resolved! label Jan 6, 2024

TheBusyBot added the ⚡ Merge Conflicts This Pull Request has merged conflicts which need to be resolved! label Jan 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom research unlock #3942

Custom research unlock #3942

StarWishsama commented Aug 9, 2023 •

edited

Loading

github-actions bot commented Aug 9, 2023

Sefiraat commented Aug 9, 2023

J3fftw1 left a comment

Sefiraat commented Aug 9, 2023

StarWishsama commented Aug 9, 2023

Sefiraat commented Aug 9, 2023

J3fftw1 commented Aug 9, 2023

github-actions bot commented Aug 9, 2023 •

edited

Loading

iTwins left a comment

TheBusyBiscuit left a comment

StarWishsama commented Aug 14, 2023

Boomer-1 commented Nov 23, 2023

JustAHuman-xD Nov 23, 2023

SchnTgaiSpock Nov 23, 2023

JustAHuman-xD Nov 23, 2023

StarWishsama Nov 25, 2023

J3fftw1 commented Dec 31, 2023

StarWishsama commented Dec 31, 2023

		not-enough-xp: '&4You do not have enough XP to unlock this'
		not-meet-requirement: '&4You do not meet the requirement to unlock this'

Custom research unlock #3942

Are you sure you want to change the base?

Custom research unlock #3942

Conversation

StarWishsama commented Aug 9, 2023 • edited Loading

Description

Proposed changes

Related Issues (if applicable)

Checklist

github-actions bot commented Aug 9, 2023

Sefiraat commented Aug 9, 2023

J3fftw1 left a comment

Choose a reason for hiding this comment

Sefiraat commented Aug 9, 2023

StarWishsama commented Aug 9, 2023

Sefiraat commented Aug 9, 2023

J3fftw1 commented Aug 9, 2023

github-actions bot commented Aug 9, 2023 • edited Loading

Slimefun preview build

iTwins left a comment

Choose a reason for hiding this comment

TheBusyBiscuit left a comment

Choose a reason for hiding this comment

StarWishsama commented Aug 14, 2023

Boomer-1 commented Nov 23, 2023

JustAHuman-xD Nov 23, 2023

Choose a reason for hiding this comment

SchnTgaiSpock Nov 23, 2023

Choose a reason for hiding this comment

JustAHuman-xD Nov 23, 2023

Choose a reason for hiding this comment

StarWishsama Nov 25, 2023

Choose a reason for hiding this comment

J3fftw1 commented Dec 31, 2023

StarWishsama commented Dec 31, 2023

StarWishsama commented Aug 9, 2023 •

edited

Loading

github-actions bot commented Aug 9, 2023 •

edited

Loading