hi Denis!<br><br><div>this is a very impressive commit! I'm really excited as it pushes the game much further!</div><div><br><div class="gmail_quote">On Mon, Aug 10, 2009 at 9:03 PM, <span dir="ltr"><<a href="mailto:dkasak13@users.sourceforge.net">dkasak13@users.sourceforge.net</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">Revision: 43253<br>
<a href="http://scummvm.svn.sourceforge.net/scummvm/?rev=43253&view=rev" target="_blank">http://scummvm.svn.sourceforge.net/scummvm/?rev=43253&view=rev</a><br>
Author: dkasak13<br>
Date: 2009-08-11 04:03:22 +0000 (Tue, 11 Aug 2009)<br>
<br>
Log Message:<br>
-----------<br>
Added dialogue support.</blockquote><div><br></div><div>great job!!!</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">Modified: scummvm/branches/gsoc2009-draci/engines/draci/game.cpp<br>
===================================================================<br>
--- scummvm/branches/gsoc2009-draci/engines/draci/game.cpp 2009-08-11 02:12:24 UTC (rev 43252)<br>
+++ scummvm/branches/gsoc2009-draci/engines/draci/game.cpp 2009-08-11 04:03:22 UTC (rev 43253)<br>@@ -61,21 +63,24 @@<br>
// Close persons file<br>
file->close();<br>
<br>
- // Read in dialog offsets<br>
+ // Read in dialogue offsets<br></blockquote><div><br></div><div>I see that you like British English :-)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
@@ -227,6 +232,19 @@<br>
Text *speech = new Text("", _vm->_bigFont, kFontColour1, 0, 0);<br>
speechAnim->addFrame(speech);<br>
<br>
+ for (uint i = 0; i < kDialogueLines; ++i) {<br>
+ _dialogueAnims[i] = _vm->_anims->addText(-10 - i, true);</blockquote><div><br></div><div>is -10 a magical constant making sure the texts have unique animation ID? maybe you should declare a constant for it and say in the header file how many numbers are allocated around it</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><br>
+ Text *dialogueLine0 = new Text("", _vm->_smallFont, kLineInactiveColour, 0, 0);<br>
+ _dialogueAnims[i]->addFrame(dialogueLine0);<br>
+<br>
+ _dialogueAnims[i]->setZ(254);<br>
+ _dialogueAnims[i]->setRelative(1,<br>
+ kScreenHeight - (i + 1) * _vm->_smallFont->getFontHeight());</blockquote><div><br></div><div>maybe you wanna increase the font height by 1 pixel before multiplication, to make sure there is a little gap between the lines</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">@@ -469,7 +501,193 @@<br>
<br>
return kObjectNotFound;<br>
}<br>
+<br>
+void Game::dialogueMenu(int dialogueID) {<br>
<br>
+ int oldLines, hit;<br>
+<br>
+ char tmp[5];<br>
+ sprintf(tmp, "%d", dialogueID+1);</blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
+ Common::String ext(tmp);<br>
+ _dialogueArchive = new BArchive(dialoguePath + ext + ".dfw");<br></blockquote><div><br></div><div>more elegant is to do all path forming at once, i.e. sprintf(tmp, "%s%d.dfw", ...), but then you need to allocate more space for tmp, of course.</div>
<div> </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">+ debugC(4, kDraciLogicDebugLevel, "Starting dialogue (ID: %d, Archive: %s)",<br>
+ dialogueID, (dialoguePath + ext + ".dfw").c_str());</blockquote><div><br></div><div>especially since here you form the path again. just reuse the same variable then</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
+ do {<br>
+ _dialogueExit = false;<br>
+ hit = dialogueDraw();<br>
+<br>
+ debug(2, "Hit: %d, _lines[hit]: %d", hit, _lines[hit]);<br>
+<br>
+ if ((!_dialogueExit) && (hit != -1) && (_lines[hit] != -1)) {</blockquote><div><br></div><div>this condition is quite complicated. I would rather test for the opposite and break, and then this true-branch need not be indented.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">+void Game::runDialogueProg(GPL2Program prog, int offset) {<br>
+<br>
+ // Mark last animation<br>
+ int lastAnimIndex = _vm->_anims->getLastIndex();<br>
+<br>
+ // Run gate program<br>
+ _vm->_script->run(prog, offset);</blockquote><div><br></div><div>fix the comment. there is not gate here</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
]+ // Delete all animations loaded after the marked one<br>
+ // (from objects and from the AnimationManager)<br>
+ for (uint i = 0; i < getNumObjects(); ++i) {<br>
+ GameObject *obj = &_objects[i];<br>
+<br>
+ for (uint j = 0; j < obj->_anims.size(); ++j) {<br>
+ Animation *anim;<br>
+<br>
+ anim = _vm->_anims->getAnimation(obj->_anims[j]);<br>
+ if (anim != NULL && anim->getIndex() > lastAnimIndex)<br>
+ obj->_anims.remove_at(j);<br>
+ }<br>
+ }</blockquote><div><br></div><div>I think I have seen a similar loop before and maybe more than once. perhaps they all have something in common and can be refactored into a separate method.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Modified: scummvm/branches/gsoc2009-draci/engines/draci/game.h<br>
===================================================================<br>
--- scummvm/branches/gsoc2009-draci/engines/draci/game.h 2009-08-11 02:12:24 UTC (rev 43252)<br>
+++ scummvm/branches/gsoc2009-draci/engines/draci/game.h 2009-08-11 04:03:22 UTC (rev 43253)<br>
@@ -27,6 +27,7 @@<br>
#define DRACI_GAME_H<br>
<br>
#include "common/str.h"<br>
+#include "draci/barchive.h"<br>
#include "draci/script.h"<br>
#include "draci/animation.h"<br>
#include "draci/sprite.h"<br>
@@ -68,6 +69,10 @@<br>
kDefaultRoomMap = -1<br>
};<br>
<br>
+enum {<br>
+ kNoDialogue = -1, kDialogueLines = 4<br>
+};</blockquote><div><br></div><div>there is one occurrence of raw constant 4 in dialogueDraw() </div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
@@ -146,6 +151,14 @@<br>
byte _fontColour;<br>
};<br>
<br>
+struct Dialogue {<br>
+ int _canLen;<br>
+ byte *_canBlock;</blockquote><div><br></div><div>since this pair of variables for a mathematical expression is stored in many objects in the game, maybe it would be worth wrapping them by a simple class or struct, similarly to GPL2Program.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">@@ -286,6 +304,22 @@<br>
<br>
int _currentIcon;<br>
<br>
+// HACK: remove public when tested and add getters instead<br>
+public:<br>
+ uint *_dialogueOffsets;<br>
+ int _currentDialogue;<br>
+ int *_dialogueVars;<br>
+ BArchive *_dialogueArchive;<br>
+ Dialogue *_dialogueBlocks;<br>
+ bool _dialogueBegin;<br>
+ bool _dialogueExit;<br>
+ int _currentBlock;<br>
+ int _lastBlock;<br>
+ int _dialogueLines;<br>
+ int _blockNum;<br>
+ int _lines[4];</blockquote><div><br></div><div>suggest to rename _lines to something more specific. this is a very specific variable, so it's confusing to have such a general name.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
+ Animation *_dialogueAnims[4];</blockquote><div><br></div><div>here and above, use kDialogueLines</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Modified: scummvm/branches/gsoc2009-draci/engines/draci/script.cpp<br>
===================================================================<br>
--- scummvm/branches/gsoc2009-draci/engines/draci/script.cpp 2009-08-11 02:12:24 UTC (rev 43252)<br>
+++ scummvm/branches/gsoc2009-draci/engines/draci/script.cpp 2009-08-11 04:03:22 UTC (rev 43253)<br>@@ -218,6 +218,27 @@<br>+int Script::funcBlockVar(int blockID) {<br>
+ return _vm->_game->_dialogueVars[_vm->_game->_dialogueOffsets[_vm->_game->_currentDialogue] + blockID];</blockquote><div><br></div><div>why do you use blockID alone here, whereas you use _lastBlock + 1 in game.cpp:549 ?</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">+int Script::funcHasBeen(int blockID) {<br>
+ return _vm->_game->_dialogueVars[_vm->_game->_dialogueOffsets[_vm->_game->_currentDialogue] + blockID] > 0;</blockquote><div><br></div><div>ditto</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
@@ -373,9 +394,12 @@<br>
return;<br>
}<br>
<br>
- int objID = params.pop() - 1;<br>
- int animID = params.pop() - 1;<br>
+ int objID = params.pop();<br>
+ int animID = params.pop();<br>
<br>
+ objID -= 1;<br>
+ animID -= 1;</blockquote><div><br></div><div>why this? it doesn't make anything more readable and you still use the simpler (and preferred, by me) form at many other places.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
+void Script::resetBlock(Common::Queue<int> ¶ms) {<br>
+ int blockID = params.pop();<br>
+<br>
+ _vm->_game->_dialogueVars[_vm->_game->_dialogueOffsets[_vm->_game->_currentDialogue]+blockID] = 0;</blockquote><div><br></div><div>not sure if you should use blockID or subtract 1 from it, due to 1-indexing vs. 0-indexing, and the confused addition of +1 above</div>
<div> </div></div>-- <br>Robert Špalek <<a href="mailto:rspalek@gmail.com">rspalek@gmail.com</a>><br>
</div>