feat(JOML): Migrate methods of WorldProvider #3899
Conversation
|
This pull request introduces 1 alert when merging 1f51efb into 12f4964 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging 220b894 into 12f4964 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging 2f4abaa into 12f4964 - view on LGTM.com new alerts:
|
| @@ -269,6 +269,16 @@ public void testEntityBecomesTemporaryWhenChangedFromAKeepActiveBlock() { | |||
| assertFalse(blockEntity.isActive()); | |||
| } | |||
|
|
|||
| @Test | |||
| public void testEntityBecomesTemporaryWhenChangedFromAKeepActiveBlockJoml() { | |||
| * @param pos The position | ||
| * @return Whether the given block is active | ||
| */ | ||
| boolean isBlockRelevant(Vector3ic pos); |
There was a problem hiding this comment.
Should we consider (in general) adding default interface implementations when adding new methods like this? That could avoid having to update some things in modulespace. In this case I'm not sure it makes sense as I only see one non-engine usage over in FlexiblePathfinding.
There was a problem hiding this comment.
It can't hurt at the moment but a lot of this is temporary and all the old methods are going to go away anyways. It can't hurt to implement it even if the use case is limited.
| @Override | ||
| public boolean isBlockRelevant(Vector3f pos) { | ||
| return isBlockRelevant(new Vector3i(pos, RoundingMode.HALF_UP)); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isBlockRelevant(Vector3fc pos) { | ||
| return isBlockRelevant(new org.joml.Vector3i(pos, org.joml.RoundingMode.HALF_UP)); |
There was a problem hiding this comment.
Explicit package like this makes me sad - can we avoid it via imports or is something in the way of that?
There was a problem hiding this comment.
nope, Vector3i conflicts with Vector3i from termath. I expect this to be temporary until we get to the point of removing all the old methods. we can use an alias but that somehow feels worse.
skaldarnar
added
Api
Type: Improvement
skaldarnar
changed the title
skaldarnar
added
Topic: Architecture
I will start going through some of the more major systems in Terasology and provide JOML alternatives that we can start to use in place of the teramath implementations. we should be able to start removing the termath methods when these endpoints get updated in module space. I think I will start to do this for some of the more major systems that terasology uses to make the transition easier. Components are another issue that will be harder to address.
These methods are now backed by their joml equivalent so any actions on a given block should be the same from before. I expect breaking blocks and placement of blocks should still work the same regardless.