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

[Breaking all] Gestalt v7 re-integration.... #4144

Closed
wants to merge 21 commits into from

Conversation

DarkWeird
Copy link
Contributor

@DarkWeird DarkWeird commented Sep 7, 2020

Contains

Re-integration gestaltv7

  • Changing engine module to package module. ( classpath modules dropped)
  • engine moved in self package (org.terasology.engine now)
  • create syntetic package module for nui (easier then write all in whitelist)
  • Update gestalt to 7.0.5
  • Update gson to 2.8.5
  • Update TeraNui to 2.0.0
  • Update Guava to 29.0-jre
  • Update Reflections to org.terasology:reflections:0.9.12-MB (gestalt build it)
  • Reimplemented Module download (gestalt drop BaseModule class)
  • Restore Module loading on gestalt v7
  • Use AutoreloadingAssetManager (seems asset must reload easier now)
  • Replace condition Module#isSourceModule to checking Module's files source == DirectoryFileSource - must be equal ( affected WorldGeneratorManager)
  • add Gestalt's classes to whitelist
  • remove duplication classes from org.terasology.input(org.terasology.engine.input now). We have this classes in TeraNUI
  • move cacheReflections task on after classes task (partial resolve Substantial performance issues in latest JS, possibly spawning at 0,0,0 in multiplayer #4126 (comment)) (reflections gathering for running from sources now)
  • remove cleanReflections after jar task
  • renamed cacheReflections's artifact reflections.cache to manifest.json and serializer changed to Json for using gestaltv7's default values

Fixes #2445

Testing

  1. Check annotation marked classes reloading (e.g. WorldGenerator)

    1. Run game from idea.
    2. create game, exit
    3. create dummy world generator in active module
    4. create game with advanced settings,
    5. try found your new world generator
  2. Check assets reloading

    1. Run game from idea. (in debug mode)
    2. create game
    3. change any asset of active module
    4. recompile classes
    5. exit game and load game
    6. asset must be edited in game.
  3. Check module downloading

    1. Run game
    2. Press New game
    3. Press Create game
    4. Press Advanced game setup
    5. try download any module.
  4. Fast check for 70% sure that game works

    1. run gradlew test
    2. try found failed tests ;)
  5. Check guava 29.0-jre :

    1. Play with MasterOfOreon ( minions uses guava's changed Futures for pathfinding)
    2. make them to use pathfinding

6.* Check what everything works fine:

  1. Play with all modules
  2. check all features
  3. if you found bug, be sure that bug from this PR :)
    • impossible

Outstanding before merging

  • Engine compiles and runs.
  • Reimplement in-game module downloading.
  • [?] Check and fix from server module downloading.
  • Check and fix Assets.
  • Check and fix module loading.
  • Check and fix whitelist.
  • Fix tests.
  • Update module space

Linked PRs

MovingBlocks/TeraNUI#27

@DarkWeird DarkWeird changed the title Gestalt v7 re-integration.... [Breaking all] Gestalt v7 re-integration.... Sep 9, 2020
@DarkWeird DarkWeird added the Topic: Architecture Requests, Issues and Changes related to software architecture, programming patterns, etc. label Sep 9, 2020
@4Denthusiast
Copy link
Contributor

4Denthusiast commented Sep 11, 2020

Why was moving things from org.terasology to org.terasology.engine necessary? It'll probably conflict with every other PR in a way that'll be awkward to fix, and the majority of module PRs too, and it makes it harder to find the actual changes in this PR to review it. Github isn't even able to detect the files a moved. Is there really no other way to do this?

return result;
}
}
///*
Copy link
Contributor

Choose a reason for hiding this comment

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

There must be a better way of dealing with this than commenting out the entire file. Isn't there some mechanism for ignoring tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There must be a better way of dealing with this than commenting out the entire file. Isn't there some mechanism for ignoring tests?

It is draft PR. :P

Compile time problems.

This test and ModuleListDownloader will be reimplemented for gestalt v7

@DarkWeird
Copy link
Contributor Author

Why was moving things from org.terasology to org.terasology.engine necessary? It'll probably conflict with every other PR in a way that'll be awkward to fix, and the majority of module PRs too, and it makes it harder to find the actual changes in this PR to review it. Github isn't even able to detect the files a moved. Is there really no other way to do this?

Gestalt v7 haven't classpath modules now (which works via detects ProtectedDomain of module.txt file).
Gestalt v7 uses packageModule now. It is more clean solution, really.
If I move package module back - then all classes(module's too) will be owned by engine.

I will mark changes in PRs.
99% chages not harmles and not affect functionality.

# Conflicts:
#	engine-tests/src/test/java/org/terasology/logic/location/LocationComponentTest.java
#	engine/src/main/java/org/terasology/logic/location/LocationComponent.java
#	engine/src/main/java/org/terasology/math/Direction.java
@DarkWeird DarkWeird marked this pull request as ready for review September 16, 2020 14:10
@keturn keturn self-requested a review September 20, 2020 20:02
@keturn
Copy link
Member

keturn commented Sep 24, 2020

oh geez, how am I going to review this. I'm guessing it'll be a matter of looking at some specific commits, instead of the whole batch, and/or doing a diff of some specific sub-packages?

suggestions of which packages to look at? Just as a wild guess based on what I think gestalt does: persistence, reflection, core.module? anything else?

@DarkWeird
Copy link
Contributor Author

Commit still works :D
Functional changes:
https://github.com/MovingBlocks/Terasology/pull/4144/files#diff-d068a678d72605ebbfab1729d1d8df29 - module

https://github.com/MovingBlocks/Terasology/pull/4144/files#diff-f79fded56bde13b73bbe93b818ff82f0R116-R145 - assets registering ( also several places with this chages )

https://github.com/MovingBlocks/Terasology/pull/4144/files#diff-873dc4d71db70d570c73ef0cb16a8731 - don't remove reflections after jar and reflections creates after classes ( modules)
https://github.com/MovingBlocks/Terasology/pull/4144/files#diff-1d22d9382cea237552a9f1e8e4485485 - don't remove reflections after jar and reflections creates after classes ( engine)

https://github.com/MovingBlocks/Terasology/pull/4144/files#diff-67cbca108af193bf1168895cb5237cfb - reimplement ModuleListDownloader (gestalt removed BaseModule and change ModuleMetadataJsonAdapater)

https://github.com/MovingBlocks/Terasology/pull/4144/files#diff-3e66f8ef304d0b5861aa3140ee518680 - remove ShrinkWrapFileSystem (Gestalt works only with Files instead Paths now - android compabality) (P.S. assets registring too)

https://github.com/MovingBlocks/Terasology/pull/4144/files#diff-1a57b96a1b0e27812cb3ca32cd8c8415R161-R162 - autoReload assets on EnvironmentSwitch - assetManager AutoReloadableAssetManager.

3e3faaf - nui syntetic module
b95028b - fix nui in UI assets

https://github.com/MovingBlocks/Terasology/pull/4144/files#diff-4e37f88c0233a3735d64003821a290baR119 - Replace module.IsSourceModule to this. gestaltv7 haven't this property.

https://github.com/MovingBlocks/Terasology/pull/4144/files#diff-9a8352c29b8e1414f20de160f54c8795 - Unittest package module now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Topic: Architecture Requests, Issues and Changes related to software architecture, programming patterns, etc.
Projects
None yet
4 participants