barryg613 / uqm-hd

Automatically exported from code.google.com/p/uqm-hd
0 stars 0 forks source link

Version 3: fix uio_ungetc() semantics #189

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
From my #sc2 IRC logs: (context - floated the following patch for 
consideration: https://gist.github.com/4639972)

[Saturday, January 26, 2013] [10:52:54 AM] <SvdB> regarding your save game 
patch:
[Saturday, January 26, 2013] [10:52:54 AM] <SvdB> 13:05 < SvdB> oldlaptop: does 
this yield backwards-compatible save games?
[Saturday, January 26, 2013] [10:52:54 AM] <SvdB> 13:09 < SvdB> there are some 
problems with the patch though
[Saturday, January 26, 2013] [10:52:54 AM] <SvdB> 13:09 < SvdB> not at the very 
least that the semantics of uio_ungetc() are wrong
~snip~
[Saturday, January 26, 2013] [10:54:25 AM] <SvdB> uio_ungetc() should take as 
the first parameter the character to push back, not the number of characters to 
push back

Original issue reported on code.google.com by oldlapto...@gmail.com on 4 Feb 2013 at 10:04

GoogleCodeExporter commented 8 years ago
Haha!
I misunderstood the uio_ungetc() pretty royally, it seems. 
It still looks like the backwards-compatibility fix works DESPITE my best 
efforts to screw it up :-p

Or does it for you?

Original comment by Jaakko.M...@gmail.com on 5 Feb 2013 at 7:09

GoogleCodeExporter commented 8 years ago
Now that I delved into the subject, I noticed that I had changed the contents 
of the non-implemented uio_ungetc to rewind the byte stream in r906. I had 
misunderstood the mechanism of uio_ungetc and thought that this was how it was 
supposed to work.

To clarify things, I've made a new function uio_rewind which does just that - 
rewinds the stream. Uio_ungetc is back in its original form - unimplemented. 

Fixed in r1015.

Original comment by Jaakko.M...@gmail.com on 6 Feb 2013 at 2:14

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
[Saturday, February 16, 2013] [03:31:10 PM] <SvdB_> oldlaptop: 'rewind()' isn't 
a good name for this function either... the standard C rewind() function only 
takes the stream as a parameter.
~snip~
[Saturday, February 16, 2013] [04:01:08 PM] <SvdB> actually, the entire 
function is a bit unreliable. It may work for the purpose of save game naming, 
but it is not usable in general. If you try to rewind more than the size of the 
buffer, bad things happen.
[Saturday, February 16, 2013] [04:02:16 PM] <oldlaptop> So ideally you would 
want it to be made suitable for general use?
[Saturday, February 16, 2013] [04:02:47 PM] <SvdB> yes. uio is a generic library
[Saturday, February 16, 2013] [04:03:03 PM] <SvdB> but there is no need to 
change it at all
[Saturday, February 16, 2013] [04:04:07 PM] <SvdB> imho, a better solution 
would be to use the existing uio_fseek() function
[Saturday, February 16, 2013] [04:04:48 PM] <SvdB> maybe in combination with 
ftell() (which also exists)

Original comment by oldlapto...@gmail.com on 16 Feb 2013 at 9:10

GoogleCodeExporter commented 8 years ago

Original comment by oldlapto...@gmail.com on 27 Feb 2013 at 9:47

GoogleCodeExporter commented 8 years ago
Agreed - "uio_rewind", too, is a misleading name since it has that standard c 
function counterpart.

I've changed the name of the function. However, this close to releasing the 
Beta I will adopt a "if it ain't broken, don't fix it" -policy. The function 
works for us, so it is good enough for UQM-HD - better have some ugly code 
that's guaranteed to keep people's savegames intact than break them for the 
sake of merging HD code with vanilla somewhere in the future.

However, I totally understand that the core team wants to keep the uio library 
as a general library, and add as few extraneous functions to it as possible. 
So, Oldlaptop, if you want to convert it to fseek/ftell form, go ahead and do 
it when making the UQM-HD compatible with the vanilla code. But please, do it 
only after the beta release - or if you still wanna do it before releasing the 
Beta, make really sure that every possible savegame format still works - 
vanilla UQM, Alpha and Beta.

Original comment by Jaakko.M...@gmail.com on 11 Mar 2013 at 8:29

GoogleCodeExporter commented 8 years ago
(Updated title. Gonna leave this for the final release, not for Beta.)

Original comment by Jaakko.M...@gmail.com on 9 May 2013 at 11:07