Skip to content

Add FeatureFlag API#8952

Merged
Machine-Maker merged 3 commits into
PaperMC:masterfrom
Machine-Maker:feature/feature-flag-api
Sep 7, 2024
Merged

Add FeatureFlag API#8952
Machine-Maker merged 3 commits into
PaperMC:masterfrom
Machine-Maker:feature/feature-flag-api

Conversation

@Machine-Maker
Copy link
Copy Markdown
Member

Closes #8950

My initial thoughts on API for checking if a Material or EntityType is enabled with a FeatureDependant interface.

Adds tests to check on updates when this changes.

@Machine-Maker Machine-Maker requested a review from a team as a code owner March 9, 2023 19:59
@maestro-denery
Copy link
Copy Markdown
Contributor

maestro-denery commented Mar 10, 2023

How about using @Deprecated and @APIStatus.ScheduledForRemoval() for feature flags enabling updates?

@Machine-Maker
Copy link
Copy Markdown
Member Author

How about using @deprecated and @APIStatus.ScheduledForRemoval() for feature flags enabling updates?

Deprecated seems inappropriate for this, and scheduledforremoval might work for UPDATE_1_20, but not bundle as we have no clue when that's gonna be removed. I think Experimental is the most accurate marker annotation.

JvmProfiler.INSTANCE.start(Environment.SERVER);
}

+ io.papermc.paper.world.flag.PaperFeatureFlagProviderImpl.INSTANCE.getClass(); // Paper - init FeatureFlagProvider
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this the preferred way we want to inject server singletons into the API ?
I know this is a somewhat long standing issue between services, the server interface and this one.

qixils added a commit to qixils/minecraft-crowdcontrol that referenced this pull request Apr 14, 2023
closes #128 using a temporary workaround while awaiting PaperMC/Paper#8952
Copy link
Copy Markdown
Member

@Owen1212055 Owen1212055 left a comment

Choose a reason for hiding this comment

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

Generally looks good, but think we should wait for the enum PR to allow us to have the "isEnabled" method be implemented manually to prevent the need for instance for the "FeatureElement" conversion thing.

@qixils
Copy link
Copy Markdown
Contributor

qixils commented Jun 10, 2023

Sorry, what exactly is blocking this? I don't see a "enum PR" from a glance, unless you mean the property API tagged "for: future" which I quite hope not... This is preventing me from being able to determine what items/entities are spawnable which is quite unfortunate for my use cases. I worked around it in the meantime with NMS but obviously this is not an ideal solution for cross-version compatibility among other issues. I'm hoping this could just be merged now (ideally to 1.19.4 while it's still supported ❤️) and improved later since it "generally looks good"?

@Machine-Maker
Copy link
Copy Markdown
Member Author

The enum PR was the PR previous thought to be slated by upstream for 1.20. It replaces a ton of the enums and deprecates Material.

@qixils
Copy link
Copy Markdown
Contributor

qixils commented Jun 10, 2023

Ah, upstream, I can see the hesitation then. Unfortunate but I can live with NMS for the moment if this isn't viable yet to merge. Thanks for the reasoning ❤️

@RedstoneFuture
Copy link
Copy Markdown

Would it be an option to extend the API with a method that outputs a permitted EntityType[] and Material[] for the corresponding feature-flag?

This could be helpful e.g. for tab-completions in which only the enabled blocks are to be displayed. Otherwise, you have to check all game objects individually each time.

@Machine-Maker Machine-Maker force-pushed the feature/feature-flag-api branch 2 times, most recently from f789ba0 to 4e4d350 Compare May 30, 2024 02:52
@Machine-Maker
Copy link
Copy Markdown
Member Author

Ok, revamped this and took over as much as possible from upstream's feature flag API. Added tests to ensure nothing is missed as well.

@Machine-Maker Machine-Maker requested a review from kennytv May 30, 2024 02:52
@Machine-Maker Machine-Maker force-pushed the feature/feature-flag-api branch from 4e4d350 to f95c924 Compare August 27, 2024 05:23
@Machine-Maker Machine-Maker force-pushed the feature/feature-flag-api branch 2 times, most recently from 11444f2 to 88bf4c6 Compare September 7, 2024 18:09
@Machine-Maker Machine-Maker force-pushed the feature/feature-flag-api branch from 88bf4c6 to 0bd54cb Compare September 7, 2024 20:12
@Machine-Maker Machine-Maker merged commit 925c3b9 into PaperMC:master Sep 7, 2024
@Machine-Maker Machine-Maker deleted the feature/feature-flag-api branch September 7, 2024 20:35
@ghost
Copy link
Copy Markdown

ghost commented Sep 9, 2024

you didn't list how we are supposed to fix isEnabledByFeature being marked for removal, what is the new method?

@Machine-Maker
Copy link
Copy Markdown
Member Author

The FeatureDependant and FeatureFlagSetHolder have what you want. A method on FeatureFlagSetHolder to check if a FeatureDependant is enabled. So like world#isEnabled(EntityType.PIG) or smth.

LeonTG pushed a commit to LeonTG/Paper that referenced this pull request May 17, 2026
* Add FeatureFlag API

* switch to index & move method

* fix test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Merged

Development

Successfully merging this pull request may close these issues.

API to check if a material is disabled

7 participants