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

[1.20.6] Ported Enchantment Types datalist #4827

Closed
wants to merge 7 commits into from

Conversation

Spectrall368
Copy link
Collaborator

@Spectrall368 Spectrall368 commented May 19, 2024

Ported Ench. Types mapping (since a lot has changed in enchantments, this is the best way I've found to maintain the same enchantment gui and mapping for the other versions)

@Spectrall368 Spectrall368 added the on hold Used to mark PRs that are blocked by another PR or not on the nearby roadmap label May 19, 2024
@Spectrall368 Spectrall368 changed the base branch from master to 1.20.6 May 19, 2024 12:17
@Spectrall368 Spectrall368 removed the on hold Used to mark PRs that are blocked by another PR or not on the nearby roadmap label May 19, 2024
@KlemenDEV
Copy link
Contributor

When doing porting, please respect our rules that require you to move file from ref-* generator in one step and then do changes in the other so diff can be clearly seen. Thanks!

@KlemenDEV KlemenDEV marked this pull request as draft May 19, 2024 12:33
@KlemenDEV
Copy link
Contributor

You will likely need to close PR and open new one where you first move files and then apply changes. You can store mappings file locally, revert, copy from ref, commit, and then put your file backup back and make another commit

@Spectrall368 Spectrall368 marked this pull request as ready for review May 19, 2024 13:23
Copy link
Contributor

@KlemenDEV KlemenDEV left a comment

Choose a reason for hiding this comment

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

I don't think you did move in one commit and edit in another here:

image

@KlemenDEV KlemenDEV marked this pull request as draft May 19, 2024 14:01
@Spectrall368
Copy link
Collaborator Author

I don't think you did move in one commit and edit in another here:

I did it in this commit

@KlemenDEV
Copy link
Contributor

image

you deleted the file, not moved it, that is why it is shown as deletion and not as move

@Spectrall368
Copy link
Collaborator Author

you deleted the file, not moved it, that is why it is shown as deletion and not as move

I did it with all the other PRs

@KlemenDEV
Copy link
Contributor

Well here you see move is not correctly registered so please correct this. Thanks!

@KlemenDEV
Copy link
Contributor

@Spectrall368 maybe try re-doing the PR if GH does not want to show this correctly in this one

@KlemenDEV KlemenDEV marked this pull request as ready for review May 20, 2024 20:22
@KlemenDEV
Copy link
Contributor

I am closing this PR as this should be done together with enchantment ME so the mappings make sense together with new template

@KlemenDEV KlemenDEV closed this May 21, 2024
@@ -0,0 +1,58 @@
_default: DIGGER
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this is closed, but for future reference, default value needs provide same number of mapping tables as other entries, 2 in this case

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok

@Spectrall368 Spectrall368 deleted the map_ench_types branch May 23, 2024 14:08
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.

None yet

2 participants