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

Gestaltify Everything! [Pt 2] #154

Merged
merged 10 commits into from May 31, 2017

Conversation

3 participants
@vampcat
Contributor

vampcat commented May 28, 2017

Modifies the codebase to use gestalt for loading all the assets, while also splitting up the respective items into their own files.

@GooeyHub

This comment has been minimized.

Show comment
Hide comment
@GooeyHub

GooeyHub May 28, 2017

Member

Hooray Jenkins reported success with all tests good!

Member

GooeyHub commented May 28, 2017

Hooray Jenkins reported success with all tests good!

@vampcat vampcat requested review from Rulasmur and Cervator May 28, 2017

@GooeyHub

This comment has been minimized.

Show comment
Hide comment
@GooeyHub

GooeyHub May 29, 2017

Member

Uh oh, something went wrong with the build. Need to check on that

Member

GooeyHub commented May 29, 2017

Uh oh, something went wrong with the build. Need to check on that

@GooeyHub

This comment has been minimized.

Show comment
Hide comment
@GooeyHub

GooeyHub May 29, 2017

Member

Hooray Jenkins reported success with all tests good!

Member

GooeyHub commented May 29, 2017

Hooray Jenkins reported success with all tests good!

@Cervator

Cervator approved these changes May 29, 2017 edited

Looked through the changes, even if somewhat lightly with so many files modified. Nothing particularly dramatic stuck out, a few places still have pending style fixes we can catch later.

As far as testing goes it looks good with a new ship, but if you try to use a previous ship then the game crashes with what is probably a simple bug:

java.lang.RuntimeException: Json Core:Core not found!
	at org.destinationsol.assets.AssetHelper.getJson(AssetHelper.java:119)
	at org.destinationsol.game.item.Gun$Config.load(Gun.java:170)
	at org.destinationsol.game.item.ItemManager.parseItems(ItemManager.java:134)
	at org.destinationsol.game.HardnessCalc.getShipConfDps(HardnessCalc.java:111)
	at org.destinationsol.game.ShipConfig.<init>(ShipConfig.java:42)
	at org.destinationsol.game.SaveManager.readShip(SaveManager.java:70)
	at org.destinationsol.game.SolGame.<init>(SolGame.java:159)
	at org.destinationsol.SolApplication.startNewGame(SolApplication.java:191)
	at org.destinationsol.menu.LoadingScreen.updateCustom(LoadingScreen.java:42)
	at org.destinationsol.ui.SolInputManager.update(SolInputManager.java:223)
	at org.destinationsol.SolApplication.update(SolApplication.java:150)
	at org.destinationsol.SolApplication.safeUpdate(SolApplication.java:129)
	at org.destinationsol.SolApplication.render(SolApplication.java:107)
	at com.badlogic.gdx.backends.lwjgl.LwjglApplication.mainLoop(LwjglApplication.java:215)
	at com.badlogic.gdx.backends.lwjgl.LwjglApplication$1.run(LwjglApplication.java:120)

Expecting that's an easy fix I'm conditionally approving this expecting it'll be corrected no problem :-)

Edit: Crash is even after manually deleting prevShip.ini - thought at first it was just from having old names in the file but nope.

@GooeyHub

This comment has been minimized.

Show comment
Hide comment
@GooeyHub

GooeyHub May 30, 2017

Member

Hooray Jenkins reported success with all tests good!

Member

GooeyHub commented May 30, 2017

Hooray Jenkins reported success with all tests good!

@GooeyHub

This comment has been minimized.

Show comment
Hide comment
@GooeyHub

GooeyHub May 30, 2017

Member

Hooray Jenkins reported success with all tests good!

Member

GooeyHub commented May 30, 2017

Hooray Jenkins reported success with all tests good!

@GooeyHub

This comment has been minimized.

Show comment
Hide comment
@GooeyHub

GooeyHub May 30, 2017

Member

Hooray Jenkins reported success with all tests good!

Member

GooeyHub commented May 30, 2017

Hooray Jenkins reported success with all tests good!

@Cervator Cervator merged commit 760a60e into MovingBlocks:develop May 31, 2017

2 checks passed

CodeFactor 1 issue fixed.
Details
default Build finished.
Details
@Cervator

This comment has been minimized.

Show comment
Hide comment
@Cervator

Cervator May 31, 2017

Member

Re-tested, no crash, looking good! Neat to see the item drop grammar in so quick too (although I admit not having tested deep enough to validate that part, out of time tonight, aiiee)

Merged :-)

Member

Cervator commented May 31, 2017

Re-tested, no crash, looking good! Neat to see the item drop grammar in so quick too (although I admit not having tested deep enough to validate that part, out of time tonight, aiiee)

Merged :-)

@vampcat vampcat moved this from In Progress PRs to Completed PRs in GSoC 2017 - Destination Sol Ressurected May 31, 2017

@vampcat vampcat deleted the vampcat:GestaltifyAtlas branch May 31, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment