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

Add movement threshold configuration #3820

Conversation

vectrixdevelops
Copy link
Contributor

Replaces the existing enable/disable movement checks with a customizable threshold. Using negative numbers will disable the movement checks still if desired.

Copy link
Contributor

@ImMorpheus ImMorpheus left a comment

Choose a reason for hiding this comment

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

Code looks fine! Not sure if changing existing settings should go into a new major version. cc @Zidane thoughts ?

Copy link
Member

@aromaa aromaa left a comment

Choose a reason for hiding this comment

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

As mentioned earlier, this most likely should at least respect the old configuration knobs for api-8. Or try to migrate them to the new ones.

if (SpongeGameConfigs.getForWorld(this.player.level).get().movementChecks.player.vehicleMovedTooQuickly) {
return val;
private double movementCheck$onVehicleMovedWronglyCheck(final double value) {
final double threshold = SpongeGameConfigs.getForWorld(this.player.level).get().movementChecks.movedWronglyThreshold;
Copy link
Member

Choose a reason for hiding this comment

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

This is using the player's one instead of the vehicle's. You forgot to introduce vehicle-moved-wrongly-threshold

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A previous configuration knob did not exist seperating player and vehicle and Paper/Spigot doesn't have a seperate one for each either I think. If you still think this is useful though I can add it.

Copy link
Member

Choose a reason for hiding this comment

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

I think it would be useful. It doesn't hide the fact that the same knob is used for both vehicle and players as the other one is separated like that. Also considering players and vehicles have different physics.

@vectrixdevelops
Copy link
Contributor Author

Good point. I'll add back the old configuration knobs, where if they are false it will act as though the threshold is negative (disable the check), otherwise if it's true it will enable and use the configured threshold (or the default).

@vectrixdevelops vectrixdevelops force-pushed the feature/movement-threshold-configuration branch from 10827d1 to e269d3c Compare January 22, 2023 00:10
@ImMorpheus ImMorpheus merged commit c19e80e into SpongePowered:api-8 Apr 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants