408
edits
Difference between revisions of "AGOS/TODO"
Updated my thoughts about the missing line breaks again. Mostly to correspond with the latest Simon engine changes.
Eriktorbjorn (talk | contribs) (Updated my thoughts about the missing line breaks.) |
Eriktorbjorn (talk | contribs) (Updated my thoughts about the missing line breaks again. Mostly to correspond with the latest Simon engine changes.) |
||
| Line 30: | Line 30: | ||
<pre> | <pre> | ||
} else if (chr == 0 || chr == ' ' || chr == 10) { | } else if (chr == 0 || chr == ' ' || chr == 10) { | ||
uint count = (getGameType() == GType_FF) ? _printCharPixelCount : | uint count = (getGameType() == GType_FF) ? _printCharPixelCount + 1 : _printCharPixelCount; | ||
if (_printCharMaxPos - _printCharCurPos > count) { | if (_printCharMaxPos - _printCharCurPos >= count) { | ||
_printCharCurPos += | _printCharCurPos += _printCharPixelCount; | ||
</pre> | </pre> | ||
The test checks if the remaining space on the line is larger than the length of the buffered word, i.e. if the test succeeds the word will fit. In this case, <tt>_printCharMaxPos</tt> is always 360. | The test checks if the remaining space on the line is larger than the length of the buffered word, i.e. if the test succeeds the word will fit. In this case, <tt>_printCharMaxPos</tt> is always 360. | ||
At the first line break, <tt>_printCharCurPos</tt> is 325 and <tt>count</tt> is | At the first line break, <tt>_printCharCurPos</tt> is 325 and <tt>count</tt> is 40. The word does not fit, and therefore the line is broken. | ||
Where the second line break should be, <tt>_printCharCurPos</tt> is 366 and <tt>count</tt> is | Where the second line break should be, <tt>_printCharCurPos</tt> is 366 and <tt>count</tt> is 25. At first glance it looks like the test will fail, because the remaining space is negative. However, we're using ''unsigned'' variables, so what at first apperas to be a small negative value is actually a large positive one. | ||
So how did this happen? Shouldn't the line have been broken at the previous word? No, in fact it should not. At that point, <tt>_printCharCurPos</tt> is 327 and <tt>count</tt> is | So how did this happen? Shouldn't the line have been broken at the previous word? No, in fact it should not. At that point, <tt>_printCharCurPos</tt> is 327 and <tt>count</tt> is 32. The word fits, just barely, and <tt>_printCharCurPos</tt> is increased to 358. Problem is, the line break check is triggered, here as almost always, by a space. And in that case the width of that space will ''also'' be added to <tt>_printCharCurPos</tt>, bringing it up to 366. | ||
There are several possible fixes. A few that | There are several possible fixes. A few that come to mind: | ||
* Change the variables from unsigned to signed. | * Change the variables from unsigned to signed. | ||
* Check for overflow when adding the width of the space. (I don't like this one myself.) | * Check for overflow when adding the width of the space. (I don't like this one myself, but that may be what it's trying to do already when checking if <tt>_printCharCurPos == _printCharMaxPos</tt>.) | ||
* Rewrite the test so that signedness doesn't matter, e.g. <tt>if (_printCharPos + count < | * Rewrite the test so that signedness doesn't matter, e.g. <tt>if (_printCharPos + count < _printCharMaxPos)</tt> | ||
Oddly enough, the original ''does'' use unsigned variables, and take none of these precautions. Is it possible that the compiler ''Adventure Soft'' used behaved differently (buggily?), or are we missing some subtle detail...? | Oddly enough, the original ''does'' use unsigned variables, and take none of these precautions. Is it possible that the compiler ''Adventure Soft'' used behaved differently (buggily?), or are we missing some subtle detail...? | ||