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

Improve fastbreak verbose message #1851

Open
wants to merge 7 commits into
base: 2.0
Choose a base branch
from

Conversation

SergioK29
Copy link
Contributor

use consistent case, include the target block type

use consistent case, include the target block type
@SergioK29
Copy link
Contributor Author

@ManInMyVan It would be good to have a standardized verbose format (space and = separated, camel case), I did plan this but I decided against it because people (including myself) are already parsing the verbose and I don't want to break existing setups.

@ManInMyVan
Copy link
Contributor

@ManInMyVan It would be good to have a standardized verbose format (space and = separated, camel case), I did plan this but I decided against it because people (including myself) are already parsing the verbose and I don't want to break existing setups.

majority of checks use thing1=value1, thing2=value2

@ManInMyVan
Copy link
Contributor

standardized verbose format

I wonder if allowing inputting verbose as a Map<String, Object> could be a good idea

@SamB440 SamB440 added the status: review required The pull request is awaiting review label Dec 4, 2024
@SergioK29
Copy link
Contributor Author

standardized verbose format

I wonder if allowing inputting verbose as a Map<String, Object> could be a good idea

If I made a PR overhauling verbose using map instead and adding more descriptive verbose to every check, do you think you would merge it?

@ManInMyVan
Copy link
Contributor

using map instead

verbose should still allow string input

and adding more descriptive verbose to every check

these should be seperate prs

do you think you would merge it?

I can't merge prs

@ManInMyVan ManInMyVan added the status: rebase required The pull request needs rebasing onto the merge branch label Dec 9, 2024
@SergioK29
Copy link
Contributor Author

@ManInMyVan rebased

@ManInMyVan ManInMyVan removed the status: rebase required The pull request needs rebasing onto the merge branch label Dec 12, 2024
@@ -27,8 +28,11 @@ public FastBreak(GrimPlayer playerData) {

// The block the player is currently breaking
Vector3i targetBlock = null;

// The material of block the player is currently breaking
StateType targetType = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

FastBreak gets handled before updating the block, you don't need to track the block type, you can use blockBreak.block.getType()

@SamB440
Copy link
Contributor

SamB440 commented Jan 24, 2025

Status?

@SamB440 SamB440 added the status: rebase required The pull request needs rebasing onto the merge branch label Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: rebase required The pull request needs rebasing onto the merge branch status: review required The pull request is awaiting review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants