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 FeatureSet class for holding information about mapping format capabilities #92

Merged
merged 25 commits into from
Aug 25, 2024

Conversation

NebelNidas
Copy link
Member

@NebelNidas NebelNidas commented Dec 12, 2023

Mapping formats now have an associated FeatureSet instance, which provides much improved feature support declaration capabilities compared to the existing booleans.

@liach
Copy link

liach commented Mar 8, 2024

For FeatureSet: since mapping format is mainly used on reader/writer end of the visitor chain, can't this just be replaced by extra mapping flags?

@NebelNidas
Copy link
Member Author

NebelNidas commented Mar 9, 2024

The main motivation behind this addition is to allow for programmatic querying of format features. See SubsetAssertingVisitor for example. Consumer applications might also want to limit their export options available to the user depending on element support, or at least show warnings if information loss may occur when converting to a given format.

@NebelNidas NebelNidas marked this pull request as ready for review April 13, 2024 09:38
@NebelNidas
Copy link
Member Author

The tests are only failing because of #97, apart from that everything works fine

@NebelNidas NebelNidas added this to the 0.7.0 milestone Apr 13, 2024
@NebelNidas NebelNidas changed the title Add MappingFormat$FeatureSet Add FeatureSet class for holding information about mapping format capabilities Apr 19, 2024
@NebelNidas NebelNidas requested a review from liach April 20, 2024 15:04
@NebelNidas
Copy link
Member Author

NebelNidas commented Aug 25, 2024

One last question @liach: Would you prefer MappingFormat#getFeatures() or just MappingFormat#features()? Usually I'm a proponent of the get prefix, but format.features() feels more natural than format.getFeatures() IMO.

Edit: After consulting Player, I decided to go with features().

@NebelNidas NebelNidas merged commit 368598c into FabricMC:dev Aug 25, 2024
3 checks passed
@NebelNidas NebelNidas deleted the mappingformat-featuresets branch August 25, 2024 14:12
@liach
Copy link

liach commented Aug 25, 2024

JDK prefers no get prefix, but prefers to follow existing style (such as return List vs array, existing get prefix in java.lang.Class). I think get is for mutable properties (not recommended nowadays) while record-style is for immutable.

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.

3 participants