Log for on 14th June 2014:
Times are UTC Toggle Colours
06:26:39  *** Supercheese has quit IRC
06:31:38  *** tycoondemon has joined
06:34:39  *** tycoondemon2 has quit IRC
06:34:39  *** TheIJ has quit IRC
06:36:49  *** TheIJ has joined
11:18:06  *** frosch123 has joined
11:18:06  *** ChanServ sets mode: +v frosch123
11:43:25  *** Alberth has joined
11:43:25  *** ChanServ sets mode: +v Alberth
13:11:52  <Rubidium> frosch123: it's not "really" needed, but having to send someone to play nightlies for almost a year to save his savegame seems a bit sad too
13:13:40  <Rubidium> having said that, it'll be the biggest savegame "mess" and somewhat of a reason for others to say that we really should've implemented something to make it easier to "merge" two branches of savegame versions (for patch packs)
13:23:23  <planetmaker> hu?
13:27:44  <Rubidium> there was a savegame with a huge chunk of cargodist stuff, so huge it won't load anymore
13:28:32  <Rubidium> so fonsinchen changed some things to reduce the size, in that case, a factor 1000
13:29:00  <Rubidium> although the savegame reduced from 20M to 3M
13:29:17  <fonsinchen> There is also other stuff in a savegame
13:29:21  <fonsinchen> should I commit the fix?
13:29:41  <fonsinchen> And a determined person could also break it even with the fix
13:29:53  <Rubidium> (the size of the cargodist stuff)
13:30:44  <fonsinchen> We should detect that condition somehow and leave the cargodist chunk out of the savegame, subsequently producing a warning in that case
13:31:04  <fonsinchen> If you play single player without Cargodist it's no problem.
13:31:10  <Rubidium> fonsinchen: that's fine by me
13:31:12  <fonsinchen> If you play multiplayer you will desync
13:31:40  <fonsinchen> If you play with cargodist the whole thing will have to be rebuilt once you load the resulting save
13:32:11  <Rubidium> fonsinchen: you're talking about dropping the chunk completely, right?
13:32:21  <fonsinchen> yes
13:32:29  <Rubidium> and then you can get desyncs; not the current patch
13:32:36  <fonsinchen> yes
13:32:41  <Rubidium> good ;)
13:33:20  <fonsinchen> so, I'll commit it
13:35:40  *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r26646 || Logs: || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking"
13:40:12  <frosch123> can we reactivate the savegame minor version?
13:40:26  <frosch123> checking the newgrf version is quite weird
13:40:38  <fonsinchen> Rubidium: With your comment in the thread you're implying that there is an assert for chunk size somewhere in save code, right?
13:40:47  <fonsinchen> Do you remember where that is?
13:42:03  <fonsinchen> saveload.cpp:706 seems to fit ...
13:44:22  <Rubidium> fonsinchen: yep, that's about it
13:45:04  <planetmaker> what do you need the savegame minor version for, frosch123 ? Also for this issue?
13:45:09  <fonsinchen> There is another one in saveload.cpp:864 - Now if I understood the saveload code ...
13:45:32  <frosch123> planetmaker: to insert a savegame version between current stable and nightlies
13:47:14  <Rubidium> fonsinchen: I doubt that one will be triggered as you seem to be using CH_ARRAY in linkgraph saving
13:47:28  <planetmaker> hm, ah, I see. currently we can't savegame bump stables
13:48:36  <Rubidium> frosch123: the minor version doesn't propagate into all the saveload version checks for added/removed savegame data
13:49:17  <Rubidium> unless... you imply you want to use the idea/workaround I posted yesterday and replace the NewGRF version check with the minor version number
13:49:24  <Rubidium> in that case... that might work ;)
13:58:51  <fonsinchen> Probably we could hook into SlAutolength to catch too long chunks: _sl.obj_len should be < (1 << 28), right?
14:03:42  <frosch123> Rubidium: i actually do not know where you want to get the newgrf version from. from the gamelog? then i think using the minor version is way less obscure
14:04:41  <Rubidium> yeah, it would've been the gamelog... and yes, minor version is less obscure; didn't think of that
14:16:15  <fonsinchen>
14:16:33  <fonsinchen> Prevents it from creating broken saves and gives the user an early warning
14:19:45  <planetmaker> not sure it's very helpful, though. It's like a "now you will loose" warning
14:20:08  <planetmaker> or is there anything the user could do to subsequently rescue his savegame?
14:20:23  <fonsinchen> They could remove some stations
14:20:30  <fonsinchen> (in the link graph case)
14:20:43  <fonsinchen> I'm sure there are other ways to bloat savegame chunks, though.
14:20:53  <fonsinchen> So this might be a good idea anyway.
14:26:11  <fonsinchen> Or we could try to change the gamma thing to support more lengths. Why is it restricted to 1 << 28 anyway?
14:27:07  <LordAro> better than crashing though, surely?
14:27:23  <LordAro> wait, it never crashed, nvm
14:27:54  <frosch123> before it crashed on load resp. prevented it from loading
14:28:01  <frosch123> with that diff it prevents you from saving
14:28:48  <frosch123> i wonder whether it is reasonable to skip chunks on savnig to save at lease something, or whether those savegame would likely be so ridiculously broken that there is no point in them
14:29:34  <frosch123> but well, in either case, i doubt the user would know what to do
14:29:54  <frosch123> "chunk too large" does not say "delete some stations" :p
14:30:23  <frosch123> actually, a broken savegame would allow us to debug the case
14:30:49  <planetmaker> we do have one, don't we?
14:30:50  <frosch123> while a simple error message without even the chunk name and no crash.sav is pretty useless
14:31:04  <frosch123> fonsinchen: so, actually i think that message is not a good idea
14:31:10  <frosch123> better create a broken savegame than no savegame
14:31:16  <fonsinchen> hrm, OK
14:31:47  <fonsinchen> I think we could extend the gamma thing to do 32 bits, though.
14:31:53  <frosch123> it does not help the user in any way, but it harms the information that is available for debugging
14:32:05  <fonsinchen> then we're save until someone comes up with a way to save 4GB of link graph
14:34:38  <frosch123> @calc 64000*(18 + 20*18) / 1024/1024
14:34:38  <DorpsGek> frosch123: 23.0712890625
14:34:43  <frosch123> @calc 64000*(18 + 100*18) / 1024/1024
14:34:43  <DorpsGek> frosch123: 110.961914062
14:34:48  <frosch123> @calc 64000*(18 + 1000*18) / 1024/1024
14:34:48  <DorpsGek> frosch123: 1099.73144531
14:35:10  <frosch123> 1000 links for every station? :p
14:35:57  <frosch123> but ok, increasing it likely does not hurt, though i doubt there is any legit usecase for it
14:37:38  <fonsinchen> We can even increase it further for sizeof(size_t) > 4
14:37:48  <fonsinchen> but that wouldn't be very portable, I guess
14:38:30  <frosch123> it's still uncommon for personal computers to have more than 16 GB :p
14:39:16  <planetmaker> 16GB should be enough for everyone ;)
15:10:54  <fonsinchen>
15:11:44  <fonsinchen> With that (and some hackery for transient states) I can save and reload the Megaloman game
15:11:57  <fonsinchen> even without the link graph size reduction
17:46:16  *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r26647 || Logs: || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking"
18:07:10  <Rubidium> <- updated version
18:07:20  <Rubidium> <- changes for trunk
18:07:27  <Rubidium> <- changes for 1.4
18:07:46  <Rubidium> seems to work for what I could see
18:22:11  <frosch123> r26636 needs also backporting then
18:27:31  <Rubidium> added
18:29:08  <Rubidium> nice catch ;)
18:29:43  <frosch123> looks fine to me :)
19:08:42  <fonsinchen> Rubidium: Why exactly are you doing this? My patch makes it work for up to 4GB chunk sizes. If someone has a link graph like that they'll definitely run into other problems before.
19:32:01  *** Supercheese has joined
19:32:01  *** ChanServ sets mode: +v Supercheese
20:28:28  *** DorpsGek has quit IRC
20:30:39  *** DorpsGek has joined
20:30:40  *** ChanServ sets mode: +v DorpsGek
21:12:28  *** Alberth has left
22:26:50  *** frosch123 has quit IRC
22:44:23  <fonsinchen> With you should be able to do link graphs of about 16k stations without running into the chunk size problem.
22:44:42  <fonsinchen> That should be enough for everyone - at least until we release 1.5
22:48:14  <planetmaker> fonsinchen: not sure I like the way to nest comments and code in  src/saveload/saveload.cpp:682 there

Powered by YARRSTE version: svn-trunk