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

feat(JOML): migrate SimpleFarming #101

Merged
merged 7 commits into from
Nov 5, 2020
Merged

Conversation

pollend
Copy link
Member

@pollend pollend commented Oct 5, 2020

Copy link
Contributor

@skaldarnar skaldarnar left a comment

Choose a reason for hiding this comment

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

Here, have a few comments on this one 🙃 Overall looks fine, I'm just wondering how much effort we should put into setting all the interfaces straight to an idiomatic use of JOML…?

Assertions.assertTrue(dropSpy.getEntityRefs().stream().anyMatch(k -> k.getParentPrefab().getName().equals(component.produce)));

// check if bush is at max growth state
assertEquals(component.growthStages.size() - 2, component.currentStage);
Copy link
Contributor

Choose a reason for hiding this comment

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

So, the max growth stage is one state before the final growth state? This is confusing 🙄

Comment on lines +297 to +298
Block belowBlock = worldProvider.getBlock(position.add(0,-1,0));
position.add(0,1,0);
Copy link
Contributor

Choose a reason for hiding this comment

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

I can already see how error prone this is 🙈

@@ -298,11 +288,11 @@ public void onPlantDestroyed(DoDestroyPlant event, EntityRef entity, BushDefinit
*
* @param bushComponent the bush component of the entity
*/
private void onBushDestroyed(Vector3i position, EntityRef bush, BushDefinitionComponent bushComponent) {
private void onBushDestroyed(Vector3ic position, EntityRef bush, BushDefinitionComponent bushComponent) {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

pollend and others added 5 commits October 15, 2020 21:17
…itySystem.java

Co-authored-by: Tobias Nett <skaldarnar@googlemail.com>
…itySystemTest.java

Co-authored-by: Tobias Nett <skaldarnar@googlemail.com>
…itySystemTest.java

Co-authored-by: Tobias Nett <skaldarnar@googlemail.com>
…itySystemTest.java

Co-authored-by: Tobias Nett <skaldarnar@googlemail.com>
@jdrueckert jdrueckert changed the title Feature/joml migration feat(JOML): migrate SimpleFarming Oct 30, 2020
@jdrueckert jdrueckert merged commit 6a7c5ae into develop Nov 5, 2020
@jdrueckert jdrueckert deleted the feature/joml-migration branch November 5, 2020 19:37
jdrueckert pushed a commit to Terasology/AdditionalRails that referenced this pull request Nov 5, 2020
jdrueckert pushed a commit to Terasology/MetalRenegades that referenced this pull request Nov 5, 2020
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