Skip to content

Implement blocks ticking per world API #6204

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

Open
wants to merge 5 commits into
base: minor-next
Choose a base branch
from

Conversation

ShockedPlot7560
Copy link
Member

@ShockedPlot7560 ShockedPlot7560 commented Dec 20, 2023

Introduction

We already support to change the random tick speed in server, however we can't control which world or change it at runtime. This PR fix that by providing an API and adding new configuration key.

Relevant issues

Changes

API changes

Adding the following functions to World:

  • setTickedBlocksPerSubchunkPerTick
  • getTickedBlocksPerSubchunkPerTick

Adding a new event WorldTickedBlocksChangeEvent which is fired when the value of tickedBlocksPerSubchunkPerTick change (not at the initialisation).

Backwards compatibility

No

Follow-up

Change the configuration nested the worlds per-tick and the default value under a blocks-per-subchunk-per-tick

@ShockedPlot7560 ShockedPlot7560 added Category: API Related to the plugin API Category: Core Related to internal functionality Type: Enhancement Contributes features or other improvements to PocketMine-MP labels Dec 20, 2023
@CicciogamerRrr

This comment was marked as spam.

Copy link
Member

@dktapps dktapps left a comment

Choose a reason for hiding this comment

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

I feel like the method names are too esoteric for anyone to ever actually use them. Can we simplify these names at all without losing the meaning?

Joshy3282 added a commit to Joshy3282/PocketMine-MP that referenced this pull request Oct 3, 2024
@dktapps dktapps added the Opinions Wanted Request for comments & opinions from the community label Nov 14, 2024
@ShockedPlot7560
Copy link
Member Author

Can we simplify these names at all without losing the meaning?

The only names i see would be:

  • initTickedBlocksPerSubchunkPerTickinitSubchunkTicks
  • setTickedBlocksPerSubchunkPerTicksetSubchunkTicks
  • getTickedBlocksPerSubchunkPerTickgetSubchunkTicks

But we will need any docs to precise this is the number of block ticked inside each subchunk each tick. But for now, I don't see any shorter name.

@dktapps
Copy link
Member

dktapps commented Nov 18, 2024

I feel like those are too ambiguous too.

@ShockedPlot7560
Copy link
Member Author

I can't think of any other name without losing the original meaning of the function.

@dktapps
Copy link
Member

dktapps commented Nov 18, 2024

Perhaps the issue here is that we're exposing something that is too "internal". "Random tick speed" would be the most intuitive name, but the problem is that, because of its usage, the values don't map onto what users would expect.

We could expose a random tick speed that is just this value divided by 3, but we'd probably have to make it a decimal value to allow the level of granularity that the current setting has. This then exposes all kinds of edge cases like what happens if somebody sets a tick speed of 1.5. We can't tick 4.5 blocks per tick without a lot more complexity.

@ShockedPlot7560
Copy link
Member Author

ShockedPlot7560 commented Feb 17, 2025

Come to think of it, isn't it possible to add a warning in the console if the final value is a decimal, saying that it has been rounded up or down?

@dktapps
Copy link
Member

dktapps commented Feb 17, 2025

No one reads warnings, and even if they do, they won't know why it's being emitted if a plugin triggers it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: API Related to the plugin API Category: Core Related to internal functionality Opinions Wanted Request for comments & opinions from the community Type: Enhancement Contributes features or other improvements to PocketMine-MP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants