On Tue, Aug 11, 2009 at 12:35 PM, Denis Kasak <span dir="ltr"><<a href="mailto:denis.kasak@gmail.com">denis.kasak@gmail.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Yes, it's a magical constant. The code from yesterday isn't very<br>
polished; it was a real ordeal getting it to work and I was really<br>
tired afterwards so I decided to do the polish today. There are many<br>
such aesthetically unappealing quirks in the current implementation<br>
(for instance, everything dialogue related is public now). I'll fix<br>
that today.<br>
<div class="im"></div></blockquote><div><br></div><div>sure, I'm aware of that. I just thought it may be useful to have a list of things to clean.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im">I initially thought that this will be needed everywhere, however gaps</div>
seem to be incorporated in the fonts already. Are you suggesting this<br>
as an aesthetic improvement? I'm asking because in the original<br>
player, there is no gap explicitly added.<br>
<div class="im"></div></blockquote><div><br></div><div>all right, follow the old player then. I wasn't aware the gap is already a part of the font.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im">> this condition is quite complicated. I would rather test for the opposite</div><div class="im">
> and break, and then this true-branch need not be indented.<br>
<br>
</div>Copied from the original.</blockquote><div><br></div><div>ok, good enough for now, but nice to clean it up a bit. I think the original code is pretty horrible at some places and it would a pity to mici it at all costs.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"> I'm not sure I even understand it<br>
completely, particularly because the original engine checks for 0 and<br>
I couldn't deduce what should be 0-indexed and what not, so this is<br>
largely a result of trial-and-error. I'll see what I can do.</blockquote><div><br></div><div>I was actually thinking similarly that the condition doesn't make much sense!! maybe it isn't. the old contains a few hacks that had been needed at some point due to crappy libraries and later ceased to be necessary, however we left them there just to make sure. there are often comments saying {this may not be necessary?}.</div>
<div><br></div><div>I don't know what to do to make your life as easy as possible. what about importing such fishy code and right away commenting it out in your source code labeled by some sign so that if things break down you know what kind of trick MAY fix it?</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div class="im">> suggest to rename _lines to something more specific. this is a very</div><div class="im">
> specific variable, so it's confusing to have such a general name.<br>
<br>
</div>Most of these identifiers can have a better name. For instance,<br>
_dialogueLines should actually be something suggestive of a count and<br>
_lines could be named _dialogueLines instead or something similar. I<br>
only used these names while implementing it to maintain similarity to<br>
the original so I have one less thing to remap mentally.<br>
<div class="im"></div></blockquote><div><br></div><div>agreed. just label this e-mail with TODO for later so that once your code is stable and no correspondence to the old code is needed, you can go through all suggested renames and clean-ups and perform them.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div class="im">> why do you use blockID alone here, whereas you use _lastBlock + 1 in</div><div class="im">
> game.cpp:549 ?<br>
<br>
</div>Most likely an error on my part. Now that everything is at least<br>
partially working, I'll have to re-read my implementation to fix such<br>
errors.<br>
<div class="im"></div></blockquote><div><br></div><div>yes, I'm quite confident that you have an error there. please grep for "Offset" (there are not too many occurrences) and verify them all at the same time.</div>
<div> </div></div>-- <br>Robert Špalek <<a href="mailto:rspalek@gmail.com">rspalek@gmail.com</a>><br>