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

Crash on loading game saved in build 3419 [$15] #12945

Closed
Coolthulhu opened this issue Jul 15, 2015 · 38 comments

Comments

Projects
None yet
@Coolthulhu
Copy link
Contributor

commented Jul 15, 2015

Seems to always reproduce: save a game in 3419 (the build that introduced new overmap saves) and load it, the game will crash during loading. Works with freshly created worlds.

Did you help close this issue? Go claim the $15 bounty on Bountysource.

@Rivet-the-Zombie

This comment has been minimized.

Copy link
Member

commented Jul 15, 2015

Works with freshly created worlds.

I don't think I understand what you're saying here.

Do you mean 'everything works properly with freshly created worlds' or 'this bug also occurs when using freshly created worlds'?

@i2amroy

This comment has been minimized.

Copy link
Member

commented Jul 15, 2015

According to the forum, "this bug also occurs when using freshly created worlds".

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Jul 15, 2015

I mean: start 3419, generate fresh world, save, load, crash.

@Rivet-the-Zombie

This comment has been minimized.

Copy link
Member

commented Jul 15, 2015

Ouch.

@BevapDin

This comment has been minimized.

Copy link
Contributor

commented Jul 15, 2015

I can't reproduce this here on 32-bit Linux curses. Loading, saving, loading again works fine. Loading old saves works. Loading old saves, saving and reloading them works. Somebody has a backtrace?

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Jul 15, 2015

Could be something windows specific then. File access?

EDIT: Building a debug build, but it'll take me an hour or more.

@macrosblackd

This comment has been minimized.

Copy link
Contributor

commented Jul 15, 2015

I'll try a linux curses build when I get home from work.

On 7/15/15, Coolthulhu notifications@github.com wrote:

Could be something windows specific then. File access?


Reply to this email directly or view it on GitHub:
#12945 (comment)

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Jul 15, 2015

Weird, gdb crashes on me when I try to debug the binary (at "loading symbols").

@karlnp

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2015

I'll poke at it when I'm home.

@drbig

This comment has been minimized.

Copy link
Member

commented Jul 16, 2015

Seems to work fine here with linux x64 tiles build at 8333882. And recent jenkins builds are at 3425... can someone on Windows confirm it works?

@narc0tiq

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2015

Probably fixed by #12948?

@karlnp

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2015

sorry, didn't manage to get to it. if some other intrepid windows haver doesn't get to it I'll confirm in eight hours or so.

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Jul 16, 2015

Still crashes for me in 3425. Windows 7, 32 bit build of the game.

@drbig

This comment has been minimized.

Copy link
Member

commented Jul 16, 2015

3425 64bit tiles build on Win XP 64 (in a VM), play now, save, load, crash:

That's no fun!

I guess I'm contacting the support team as per instructions at least... Anyone any ideas how to get any kind of useful information out of it?

@drbig

This comment has been minimized.

Copy link
Member

commented Jul 16, 2015

Tried the same on 32bit build, and got:

Huh?

So I did hit spacebar and then it crashed. Also 4 million characters long line sounds fishy?

EDIT: debug.log here - not that it looks useful, except maybe for the GAME : GROUP_FUNGI: invalid population here: -4.52622e-018 spam.

@BevapDin

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2015

@drbig Can you upload the save (the o.0.0 file should be enough). It looks like either the JSON is not correctly written or not correctly read.

@drbig

This comment has been minimized.

@BevapDin

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2015

Well, the save loads fine here on Linux. it also looks fine. The longest line has actually 4050513 characters as reported by wc -L and it's the one that contains the monster groups.

The displayed error context is a bit of, the monster group at (384,351,0) is the second to last one. The line break is after the next monster group. The reported error location 24:4050513 is the last character on that line.

My log does not contain any entries of the form GAME : GROUP__, no GROUP__ messages at all. And that is as expected (the entries appear only if a group with a radius > 1 has been split into several smaller ones, but all the groups in the save should already have radius 1).

@drbig

This comment has been minimized.

Copy link
Member

commented Jul 16, 2015

So I understand that means at least: writing saves on Windows works correctly, so the problem is with loading them.

Given it works on linux could this be some bug in the Microsoft's C++ runtime?

@kevingranade

This comment has been minimized.

Copy link
Member

commented Jul 16, 2015

@drbig

This comment has been minimized.

Copy link
Member

commented Jul 16, 2015

Possibly it's doing something readline-like

I do remember you mentioning readline some time ago.

And out of curiosity: why the monstrous lines in the first place? I agree saves are not meant to be hand-edited, but still?

@drbig drbig added the OS: Windows label Jul 16, 2015

@karlnp

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2015

Linux and Windows have fundamentally different approaches to things like line lengths, so (much like npm's huge chain of directories which exceed the max length for a given path on windows) Linux-appropriate things that don't work on Windows sometimes go unnoticed.

@drbig

This comment has been minimized.

Copy link
Member

commented Jul 16, 2015

@karlnp indeed. Hence the new Windows label and general appreciation of people trying to keep tabs on CDDA on Windows that know what they're doing :)

@karlnp

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2015

I've actually been getting similar runtime errors for a few days, including when profiling, so it's entirely possible that this is related to that, but (as in the screenshots) the stack traces have been blank, so I can't say for sure.

@DanmakuDan

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2015

During debugging in Windows, ifstream->tellg() is spontaneously reporting incorrect values when JSON array index positions are being captured. The array index is used later when converting the string into a value. My error was that the value 400 in one array was only visible as "00", triggering a float value formatting error. I found mostly complaints about \r\n differences when searching for the reason, but this is happening near the end of the 3-4 MB line for the enemy groups. The solution that worked for me is to open files in std::ifstream::binary mode instead of the default text mode:

Replace
fin.open(terfilename.c_str());
in void overmap::open() [src\overmap.cpp] with
fin.open(terfilename.c_str(),std::ifstream::binary);

and probably any other files that are getting opened and handled by JSON in Windows.

@drbig

This comment has been minimized.

Copy link
Member

commented Jul 16, 2015

@DanmakuDan do you prefer to PR that yourself (for credit), or are you ok with someone else PRing (giving credit too would be proper also)?

@karlnp

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2015

I'll post a small bounty, since that's going to be a pain in the ass.

@drbig

This comment has been minimized.

Copy link
Member

commented Jul 16, 2015

If std::ifstream::binary is all that's needed one can probably do it with sed alone.

@kevingranade kevingranade changed the title Crash on loading game saved in build 3419 Crash on loading game saved in build 3419 [$15] Jul 16, 2015

@karlnp

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2015

well I still appreciate it

@drbig

This comment has been minimized.

Copy link
Member

commented Jul 16, 2015

Indeed. @DanmakuDan you're evidently encouraged to PR fixes for it :)

@drbig

This comment has been minimized.

Copy link
Member

commented Jul 16, 2015

Btw. my question about PR was that it's a blocker so I'd rather see it fixed sooner than later.

@drbig

This comment has been minimized.

Copy link
Member

commented Jul 16, 2015

@karlnp Also please be careful on putting bounties on new issues, especially enhancements/additions - we don't want CDDA direction to be bounty-driven (we prefer it discussion-driven; bounties are to encourage effort on writing stuff that the community wants to see in).

@karlnp

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2015

Oh, certainly. I was mainly thinking about how finicky JSON parsing is, and in how many places we do it, and how evidently Linux and Windows string handling differs and so it'll probably be more of a pain then it appears to be.

@DanmakuDan

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2015

Just cleaning up some manual debug stuff that I was using to track values. Also, the Windows runtime error popup is probably because the program exited with an uncaught exception throw.

@DanmakuDan

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2015

The funny thing is, I loaded a game from earlier into 3419 and played normally, I didn't notice the issue until I saw this thread and tried the latest version.

@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2015

I can confirmed that this is still FUBAR as of build 3421.

EDIT: I am a derp. You guys already confirmed it's broken even into 3425. ;w;

@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Jul 17, 2015

And thank you to DanmakuDan, that PR seems to have unbroke it so far.

@kevingranade

This comment has been minimized.

Copy link
Member

commented Jul 17, 2015

As seems to happen more often than not, the attribution for the PR didn't quite line up, so DanmakuDan is going to need to go to BountySource and manually claim it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.