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.

Sign up for GitHub

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

Jump to bottom

refactor reading/writing into plugins #197

Merged
merged 48 commits into from
Jun 3, 2024
Merged

refactor reading/writing into plugins #197

merged 48 commits into from
Jun 3, 2024

Conversation

ix0rai
Copy link
Member

@ix0rai ix0rai commented Apr 16, 2024 •
edited
Loading

ports over FabricMC/Enigma#532 by @Juuxel, making a few changes.

  • leading dots are no longer stored in the file extensions: seemed redundant to me.
  • file extensions now support multiple different directory types, instead of hardcoding to enigma directory as the only one

OTHER CHANGES:

  • added new service type: read/write services
    • this service defines an optional reader and an optional writer for mappings
    • each service is keyed by a unique file type, of which the uniqueness is determined by the extension
    • took a lot of work but there's not much else to say here lol
  • added validation for plugin registration
    • no two services of a given type can have the same ID
    • plugins IDs must strictly adhere to the rules defined in EnigmaService javadoc
    • no two read/write services can support the exact same file type
  • service types now support being active by default
    • this type of service does not have to be explicitly defined in a profile to be active
    • being defined explicitly will override this behaviour
    • decompiler services and read/write services are active by default
  • update gradle
    • fix gradle 9 deprecation warnings
  • WRITE DOCS

@ix0rai ix0rai added enhancement New feature or request backend affects the enigma backend user interface affects the enigma ui labels Apr 16, 2024
@ix0rai
Copy link
Member Author

ix0rai commented Apr 17, 2024

not going to merge this until I have input from @IotaBread on whether FileType should be API or impl

@ix0rai ix0rai dismissed OroArmor’s stale review April 17, 2024 02:04

discussed on discord

@IotaBread
Copy link
Member

I'm thinking this could be api, and maybe we could replace MappingFormat by an interface + service (just something to hook a plugin), so more can be added by external plugins or applications, instead of having an enum with the hardcoded set of formats

@ix0rai
Copy link
Member Author

ix0rai commented Apr 17, 2024

I actually really like that suggestion -- it would allow us to do MIO support via plugin

@ix0rai ix0rai changed the title https://github.com/FabricMC/Enigma/pull/532 - juuz file extensions refactor reading/writing into plugins Apr 24, 2024
IotaBread
IotaBread reviewed May 1, 2024
View reviewed changes
Copy link
Member

@IotaBread IotaBread left a comment

Choose a reason for hiding this comment

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

Looks great so far! Most comments are nitpicks, the other 3 are more important

enigma-cli/src/main/java/org/quiltmc/enigma/command/ComposeMappingsCommand.java Outdated Show resolved Hide resolved
enigma-cli/src/main/java/org/quiltmc/enigma/command/ConvertMappingsCommand.java Outdated Show resolved Hide resolved
enigma-cli/src/main/java/org/quiltmc/enigma/command/FillClassMappingsCommand.java Outdated Show resolved Hide resolved
enigma-cli/src/main/java/org/quiltmc/enigma/command/InvertMappingsCommand.java Outdated Show resolved Hide resolved
enigma-cli/src/main/java/org/quiltmc/enigma/command/MapSpecializedMethodsCommand.java Outdated Show resolved Hide resolved
enigma/src/main/java/org/quiltmc/enigma/api/service/NameProposalService.java Outdated Show resolved Hide resolved
...ma/src/main/java/org/quiltmc/enigma/api/translation/mapping/serde/MappingFileNameFormat.java Outdated Show resolved Hide resolved
enigma/src/main/java/org/quiltmc/enigma/api/translation/mapping/serde/tinyv2/TinyV2Writer.java Outdated Show resolved Hide resolved
enigma/src/main/java/org/quiltmc/enigma/api/translation/mapping/serde/tinyv2/TinyV2Writer.java Outdated Show resolved Hide resolved
enigma/src/test/java/org/quiltmc/enigma/MappingFormatDetectionTest.java Outdated Show resolved Hide resolved
ix0rai and others added 11 commits May 1, 2024 17:07
@ix0rai
reintroduce EnigmaServiceType.create
3874707
@ix0rai
private tinyWriter constructor
85d3a19
@ix0rai
improve MappingFileNameFormat docs
17dda83
@ix0rai @IotaBread
Update enigma/src/main/java/org/quiltmc/enigma/api/service/NamePropos…
70320be 
…alService.java

Co-authored-by: Iota <[email protected]>
@ix0rai @IotaBread
Update enigma-cli/src/main/java/org/quiltmc/enigma/command/InvertMapp…
9857315 
…ingsCommand.java

Co-authored-by: Iota <[email protected]>
@ix0rai @IotaBread
Update enigma-cli/src/test/java/org/quiltmc/enigma/command/FillClassM…
e3040aa 
…appingsCommandTest.java

Co-authored-by: Iota <[email protected]>
@ix0rai
only create enigma instances once in commands
0c079ac
@ix0rai
Merge remote-tracking branch 'origin/juuxel-file-extensions' into juu…
31a0df7 
…xel-file-extensions
@ix0rai @IotaBread
Update enigma-cli/src/test/java/org/quiltmc/enigma/command/MapSpecial…
430b66f 
…izedMethodsCommandTest.java

Co-authored-by: Iota <[email protected]>
@ix0rai
use WalkFileTree
eb381ed
@ix0rai
Merge remote-tracking branch 'origin/juuxel-file-extensions' into juu…
f4c0bae 
…xel-file-extensions
@ix0rai
Copy link
Member Author

ix0rai commented May 2, 2024

I believe everything is addressed!

@ix0rai ix0rai mentioned this pull request Jun 3, 2024
Closed
@ix0rai
Copy link
Member Author

ix0rai commented Jun 3, 2024

@IotaBread do you have time to review this anytime soon? I'd like to release the update

IotaBread
IotaBread approved these changes Jun 3, 2024
View reviewed changes
Copy link
Member

@IotaBread IotaBread left a comment

Choose a reason for hiding this comment

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

Looks great! Considering everything from my previous reviews has already been solved, it's good to merge

@ix0rai
Copy link
Member Author

ix0rai commented Jun 3, 2024

🎉

@ix0rai ix0rai merged commit f2899c8 into QuiltMC:master Jun 3, 2024
2 checks passed
@ix0rai ix0rai deleted the juuxel-file-extensions branch June 3, 2024 22:55
@ix0rai ix0rai mentioned this pull request Jun 3, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IotaBread IotaBread IotaBread approved these changes

@OroArmor OroArmor OroArmor left review comments

Assignees
No one assigned
Labels
backend affects the enigma backend enhancement New feature or request user interface affects the enigma ui
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ix0rai @IotaBread @OroArmor