Fix RuntimeException while restoring a saved game #11
Conversation
…ents defined (onSetComplete, onSetActive, ...)
…ents defined (onSetComplete, onSetActive, ...)
# Conflicts: # OpenWIGLibrary/src/cz/matejcik/openwig/formats/Savegame.java
…ents defined (onSetComplete, onSetActive, ...)
…ents defined (onSetComplete, onSetActive, ...)
|
Issues
======
- Added 8
See the complete overview on Codacy |
| public ZonePoint position = null; | ||
| protected boolean visible = false; | ||
|
|
||
| public Media media, icon; |
There was a problem hiding this comment.
| //setTable(table); | ||
| } | ||
|
|
||
| public String name, description; |
There was a problem hiding this comment.
| package cz.matejcik.openwig; | ||
|
|
||
| import se.krka.kahlua.stdlib.BaseLib; | ||
| import se.krka.kahlua.vm.*; |
There was a problem hiding this comment.
| public ZonePoint position = null; | ||
| protected boolean visible = false; | ||
|
|
||
| public Media media, icon; |
There was a problem hiding this comment.
| //setTable(table); | ||
| } | ||
|
|
||
| public String name, description; |
There was a problem hiding this comment.
| Engine.log("PROP: " + toString() + "." + key + " is set to " + (value == null ? "nil" : value.toString()), Engine.LOG_PROP); | ||
| } | ||
|
|
||
| public void setMetatable (LuaTable metatable) { } |
There was a problem hiding this comment.
|
|
||
| public String name, description; | ||
| public ZonePoint position = null; | ||
| protected boolean visible = false; |
There was a problem hiding this comment.
| } | ||
|
|
||
| public String name, description; | ||
| public ZonePoint position = null; |
There was a problem hiding this comment.
bekuno
added
the
Do not merge / WIP
|
@allyourcodearebelongtous Please ignore for now the Codacy review. If you want to change some of the points of them the please create an other PR. Some hints for you. You can reopen a PR, if necessary. You need not to create a new one. Please use a dedicated branch for a PR. Then you can left master and release acording to the master/release at upstream. You have an instruction at https://github.com/cgeo/cgeo/wiki/git-beginners and cgeo/WhereYouGo#45 (comment). |
|
@bekuno
Thanks for the links. I'll try to remember that next time.
My next steps will be: work on the "actual correct fix" as described by matejcik. Until then the supposed quick fix will have to suffice.
|
|
I created a branch "restorefix" in my fork of openWIG for testing. In this branch I implemented the " slightly better fix" from matejcik, released it as v0.0.1-alpha and changed build.gradle to So I understand there is no need to build a jar file from cgeo:openWIG which is quite nice. The Crash.urwigo example cartrige from issue #8 does run fine now when restoring a saved game. I think this PR should be considered a working fix for the problem and not WIP any more. What to do now? Keep this PR and merge it into master after approval by some other testers? Switch from this PR to the changes in branch "restorefix"? I'd say keep it simple and merge from here if all is fine. I'll use "restorefix" for the "actual correct" fix (implement deserialize() in all subclasses of EventTable.java and use Engine.instance.savegame.restoreValue(in, table); in EventTable.java). But this could take a while, 'm afraid. @bekuno |
You can refer to the sort commit number, here I would suggest (if possible) to use a dedicated branch. But it is not necessary. The PR wants to change the complete content for both files. Before merging your PR I want to solve Issue #3 first. |
|
Agreed, no need to be hasty. |
|
@bekuno Maybe it's time to merge the PR now? @Lineflyer posted an issue (cgeo/WhereYouGo#367) in WhereYouGo that could be solved with this workaround. |
|
The big problem with analysing your changes is that your PR tries to replace the entire content of the two files. I checked your changes in my branch and in the second file (savegame.java) I only found changes in the formatting. This does not belong in this PR. An easy way would be to create a new branch, copy your changes to the file(s) from GitHub to your local files and create a new PR. |
|
Never mind. It's been a while and therefor I'll close this PR. After forking a new branch I'll test again. |
As suggested by mateijcik EventTable.java does not call callEvent() while deserializing. Savegame.java actually wasn't changed (it got caught in this PR because I made some changes and reverted them with this commit).
Corresponding Issue is #8