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

Lectern page tag/mech fix #2598

Merged
merged 5 commits into from
Mar 14, 2024

Conversation

heypr
Copy link
Contributor

@heypr heypr commented Feb 20, 2024

  • Coming back to an old PR of mine that I closed because I was too dumb at the time lol: Fixes the Lectern page mech and tag. #2484

  • Deprecates the lectern_page tag & mech as they are one number off, ie If a book on a lectern is on page 14, the tag will return 13. & if you want to change the page on the lectern, you have to set the number to be one number higher than the preferred page.

@mcmonkey4eva mcmonkey4eva changed the title Small tag/mech change Lectern page tag/mech fix Feb 23, 2024
@@ -320,6 +320,12 @@ public class BukkitImplDeprecations {
// Added 2023/11/16, deprecate officially by 2027
public static Warning takeExperience = new FutureWarning("takeExperience", "Using the 'take' command to take experience is deprecated in favor of the 'experience' command.");

// Added 2024/02/19, deprecate officially by 2027.
public static Warning lecternPageMech = new FutureWarning("lecternPageMech", "The mechanism 'LocationTag.lectern_page' is deprecated in favor of 'LocationTag.page'.");
Copy link
Member

Choose a reason for hiding this comment

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

should just have one dep for both instead of separate

Comment on lines 3704 to 3705
BlockState state = object.getBlockStateForTag(attribute);
if (state instanceof Lectern lectern) {
Copy link
Member

Choose a reason for hiding this comment

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

state variables are a bit redundant here (and in the mech), as they're only ever needed in the instanceof check which creates the variable that's actually used - can just in-line them like most newer tags/mechs do.

tagProcessor.registerMechanism("page", false, ElementTag.class, (object, mechanism, input) -> {
if (object.getBlockState() instanceof Lectern lectern) {
lectern.setPage(input.asInt() - 1);
object.getBlockState().update();
Copy link
Member

Choose a reason for hiding this comment

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

Need to Mechanism#requireInteger in here, probably as an early return before the state checks

object.getBlockState().update();
}
else {
Debug.echoError("The 'LocationTag.page' mechanism can only be called on a lectern block.");
Copy link
Member

Choose a reason for hiding this comment

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

Mechanism#echoError

// @name page
// @input ElementTag(Number)
// @description
// Changes the page currently displayed on the book in a lectern block.
Copy link
Member

Choose a reason for hiding this comment

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

I believe it's usually Sets?

@mcmonkey4eva mcmonkey4eva merged commit 62e96a3 into DenizenScript:dev Mar 14, 2024
1 check passed
@heypr heypr deleted the guess-whos-back-back-again branch April 8, 2024 22:19
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

3 participants