search

cutelyaware / magiccube4d

Automatically exported from code.google.com/p/magiccube4d
Other
71 stars 16 forks source link

Undoing macro then saving corrupts log file #118

Closed rzhao271 closed 8 years ago

rzhao271 commented 9 years ago

So I was testing out a new feature I was trying to implement on my fork. Anyway,

Version: was able to reproduce in original MC4D 4.0.185 Input:

  1. Make a macro and apply it onto the puzzle
  2. Ctrl+Z/undo once
  3. Save as any log file
  4. Re-open that log file.

Even saving is invalid because it fails an assertion in History.java so it doesn't save properly, leaving only a blank file that can't be parsed later.

Writing this as an issue just so others are aware of it, I guess.

cutelyaware commented 9 years ago

Can you be more specific? EG, are you saving a macro file and trying to open it as a log file or vise versa? that won't work. Certainly you're not saying that macro undo isn't incomparable with with saving log files in general, right?

On 7/22/2015 7:24 PM, Ray Z wrote:

So I was testing out a new feature I was trying to implement on my fork. Anyway,

Version: was able to reproduce in original MC4D 4.0.185 Input:

  1. Make a macro and apply it onto the puzzle
  2. Ctrl+Z/undo once
  3. Save as any log file
  4. Re-open that log file.

Even saving is invalid because it fails an assertion in History.java so it doesn't save properly, leaving only a blank file that can't be parsed later.

Writing this as an issue just so others are aware of it, I guess.

— Reply to this email directly or view it on GitHub https://github.com/cutelyaware/magiccube4d/issues/118.

rzhao271 commented 9 years ago

One of the asserts tested if History.currentnode was an actual sticker, which meant that it'd always fail at any part of the log file that had m[. It's saving the puzzle as a log file and then reopening it that fails, since when you undo a macro that was just applied an "m[." is left at the end of the history, and then it doesn't save properly.

cutelyaware commented 9 years ago

Good find, Ray! Why don't you see if you can find a nice, clean fix. It's certainly an important bug.

rzhao271 commented 9 years ago

Hmm, seems I haven't replied in a while. I wrapped one check of the assertion in a try catch so that the program will keep running and even now it seems to work. However, that solution's like magical duct tape; I'll have to take a while to get how the actual History works and code something proper there, and hopefully fix some other minor History-related issues in the process.

cutelyaware commented 8 years ago

Fixed in v4.1. commit 82c5bb72a67a7447a9b6376a2be09d0aee4e2ce1. Your magical duct tape was on the right track. The assert your test case failed was created before we started saving marks to the log files and we were "lucky" not to be triggering the test in most cases. Removing Assert(node.stickerid >= 0) was the solution.