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

First pass at an update to 1.21.2. #95

Merged
merged 9 commits into from
Nov 8, 2024

Conversation

gniftygnome
Copy link
Contributor

This PR makes the API generally work from my testing so far. The boat API is completely replaced under the hood, but the API interfaces are similar to before. The main change is treating all boat and raft types of a single wood type as a group. API calls use the group's Identifier and the API becomes the authoritative source of other identifiers. Maybe some people won't like this; I think it is worth the convenience and simplicity. One reasonable question is whether TerraformBoatData should be part of the API.

  • Make it build against 1.21.2
  • Fix broken things
  • Replace basically all of the boat API

- Make it build against 1.21.2
- Fix broken things
- Replace basically all of the boat API
@gniftygnome gniftygnome requested a review from haykam821 October 13, 2024 00:03
- PillarLogHelper.of methods removed
- PillarLogHelper.settings methods added
  They provide AbstractBlock.Settings instead of PillarLog

- Improvements to the testmod
@gniftygnome
Copy link
Contributor Author

Things I've noticed but don't yet understand:

  • Now that they have a texture, I see the custom rafts do not render quite right. There is a tiny one texture pixel high space above any edge viewed across the raft where you can see into the water. I have absolutely no idea what is going on there.
  • In just MixinEatGrassGoal, IDEA claims org.spongepowered.asm.mixin.injection.callback.LocalCapture does not exist (other classes in the same package do exist). I have tried clearing caches and restarting and so on, but it persists. However, this seems cosmetic; the build is fine and works fine...

@gniftygnome
Copy link
Contributor Author

BTW I have Traverse fully updated to use this Terraform version, so if anybody wants that for testing, let me know and I'll push it.

*
* This method should be called once for each boat type.
* Created items and entities will have identifiers similar to
* {@code id.withSuffixedPath("_boat")} and {@code id.withPrefixedPath("chest_raft/")}.
Copy link
Contributor

Choose a reason for hiding this comment

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

Would the item ID be examplemod:mahogany_boat and the entity type ID be examplemod:boat/mahogany, then? I assume the second identifier refers to the entity model layer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Now that we are doing this entirely the vanilla way, we create an item, entity type, and model layer. For our mods, there is no need to know the identifiers (although they are predictable, as needed to put the assets in the right place). I doubt anybody needs the model layer. I have wondered if others might need either the identifiers or the entity type. If we think folks might need things like these, I could refactor so the getters of TerraformBoatData are exposed in the API.

gniftygnome and others added 4 commits October 19, 2024 12:39
@gniftygnome
Copy link
Contributor Author

  • Now that they have a texture, I see the custom rafts do not render quite right. There is a tiny one texture pixel high space above any edge viewed across the raft where you can see into the water. I have absolutely no idea what is going on there.

This is fixed now in 1248758.

This is the nice clean way I wanted to do but can't because somebody decided the DFU should init before mods do...
This is the icky version I hate to have to make, but the DFU initializes way too early to do this in any nice way...
@gniftygnome gniftygnome marked this pull request as ready for review October 28, 2024 07:47
@haykam821 haykam821 self-requested a review October 31, 2024 04:51
@gniftygnome
Copy link
Contributor Author

OK, I think the API is fine; perhaps things could have better names but we can always fix that in 1.21.4 or whatever. If we do want to improve this in 1.21.2/3, it would be via API additions or internal changes, which shouldn't be disruptive. I'll release this as 12.0.0-alpha.1.

@gniftygnome gniftygnome merged commit 74c77f1 into TerraformersMC:1.21.2 Nov 8, 2024
1 check passed
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.

2 participants