Skip to content

Allow modules to load instance assets.#64

Merged
immortius merged 2 commits intoMovingBlocks:5.1.3from
flo:untested-instanced-assets-fix
Jul 31, 2017
Merged

Allow modules to load instance assets.#64
immortius merged 2 commits intoMovingBlocks:5.1.3from
flo:untested-instanced-assets-fix

Conversation

@flo
Copy link
Contributor

@flo flo commented Jul 29, 2017

This change probably fixes the instanced asset creation from issue #62

I am not sure how to configure my terasology to use the gestalt source version.

I also didn't find a proper 5.1 branch to make this PR to as Terasology still uses 5.1.2. The commit in this PR is based on the latest 5.1.3-SNAPSHOT commit.

It would be great, if you @immortius could explain me how you test a gestalt change with Terasology without building new version. Ideally this should also be documented so I created an issue for this: MovingBlocks/Terasology#3027

@flo
Copy link
Contributor Author

flo commented Jul 29, 2017

This fix is btw. needed by the Scenario module. It uses it to allow you to allow you to define "event -> actions" logic ingame without programming knowledge. For this actions you can define recursivly expressions for which it is necessary to display the same dialog multiple times.

The GSOc student @ThompsonTyler (alias cata) works on that module and @nihal111 plans to use it for his gsoc project too.

@flo
Copy link
Contributor Author

flo commented Jul 29, 2017

Ok was able to test and I can confirm that the PR indeed did fix the issue.

To test I build gestalt with "./gradlew install" and then changed in engine/build.gradle:

-    compile group: 'org.terasology', name: 'gestalt-module', version: '5.1.2'
-    compile group: 'org.terasology', name: 'gestalt-asset-core', version: '5.1.2'
+    compile group: 'org.terasology', name: 'gestalt-module', version: '5.1.3-SNAPSHOT'
+    compile group: 'org.terasology', name: 'gestalt-asset-core', version: '5.1.3-SNAPSHOT'

@Cervator Cervator changed the base branch from develop to 5.1.3 July 30, 2017 03:11
@Cervator Cervator added this to the v5.1.3 milestone Jul 30, 2017
@Cervator
Copy link
Member

Branching stuff is all set now :-)

I might simply make a new Gestalt job in Jenkins purely to build a v5.1.3

@immortius
Copy link
Member

This is a reasonable change. One request - I would like the error handling to be in line with #reload - it should get the cause from the privileged exception.

@Cervator
Copy link
Member

So essentially replace the one logger line with something like this?

logger.error("Failed to register asset '{}'", asset.getUrn().getInstanceUrn(), e.getCause());

@flo
Copy link
Contributor Author

flo commented Jul 31, 2017

@immortius done

@immortius immortius merged commit 13eb8df into MovingBlocks:5.1.3 Jul 31, 2017
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