MACVENTURE: [GSoC] Add MacVenture engine. #807

Merged
merged 204 commits into from Sep 3, 2016

Projects

None yet

4 participants

@blorente
Contributor
blorente commented Aug 15, 2016 edited

Add a ScummVM implementation of the WebVenture engine, which includes support for graphics, sound, text and savefiles.

It has been tested with Shadowgate (it is winnable), and it can detect Dèjá Vu and Dèjá Vu II, if the game files are extracted as indicated here. All of this has only been tested with the Zojoi rereleases, since I don't have access to the originals.

Known issues:

  • There is a big leak when rendering the game. It has been tracked down, more info can be found here.
  • The border files have to be added manually to the game folder. They can be downloaded here, or use the attached zip and rename it to macventure.dat.
    macventure.zip
  • There are several encapsulation faults and hacks, this is due to the opaque nature of the original implementation, since JavaScript has much looser constraints on types and variables. Therefore, functionality was favored over style for the GSoC, although all these have been marked as // HACK, and are slowly being addressed.
blorente added some commits Jun 3, 2016
@blorente blorente MACVENTURE: Add initial game files e821fd7
@blorente blorente MACVENTURE: Add game detection for Shadowgate 52d53c2
@blorente blorente MACVENTURE: Fix detection problem 6815b05
@blorente blorente MACVENTURE: Add empty engine c6070c0
@blorente blorente MACVENTURE: Add empty event loop d56f5a3
@blorente blorente MACVENTURE: Add naked window for tests 6f5997f
@blorente blorente MACVENTURE: Border Loading code 3228366
@blorente blorente MACVENTURE: Extract GUI code f095c4a
@blorente blorente MACVENTURE: Add game file manager d15e3a0
@blorente blorente MACVENTURE: Small fix in detection 2fd43c2
@blorente blorente MACVENTURE: Add basic menu loading dd072a3
@blorente blorente MACVENTURE: Add submenu loading 4611429
@blorente blorente MACVENTURE: Add menu display 88949ed
@blorente blorente MACVENTURE: Add static menus 7991b3e
@blorente blorente MACVENTURE: Add commnad window text render c09e74b
@blorente blorente MACVENTURE: Add command callback 63e4fe8
@blorente blorente MACVENTURE: Fix small border bug 1d5cbee
@blorente blorente MACVENTURE: Fix border offsets ec40b4e
@blorente blorente MACVENTURE: Add the rest of the windows 61134cf
@blorente blorente MACVENTURE: Load general settings c676bb9
@blorente blorente MACVENTURE: Change inventory to use the general settings 5368aa9
@blorente blorente MACVENTURE: Add callbacks for all windows d435230
@blorente blorente MACVENTURE: Add appropriate border bounding boxes 88e6f92
@blorente blorente MACVENTURE: Add image to self window 9564866
@blorente blorente MACVENTURE: Add save game loading 5719ea3
@blorente blorente MACVENTURE: Add generic container b3be602
@blorente blorente MACVENTURE: Fix detection 9fc9e33
@blorente blorente MACVENTURE: Add generic non-persistent container loading 56e8ac8
@blorente blorente MACVENTURE: Test and fix object loading b6a5040
@blorente blorente MACVENTURE: Add string tables 22db262
@blorente blorente MACVENTURE: Change container to return a stream b024a24
@blorente blorente MACVENTURE: Add filepath retrieval b209c52
@blorente blorente MACVENTURE: Add text huffman loading bc435b3
@blorente blorente MACVENTURE: Add object attribute retrieval a112cdc
@blorente blorente MACVENTURE: Add main loop ef5a152
@blorente blorente MACVENTURE: Add command activation 79496ea
@blorente blorente MACVENTURE: Test and complete main loop ec768fb
@blorente blorente MACVENTURE: Fix small retrieval bug da174b7
@blorente blorente MACVENTURE: Fix (another) small retrieval bug c42451b
@blorente blorente MACVENTURE: Remove unused variables aae8378
@blorente blorente MACVENTURE: Add text decoding (without composite) 9c10b43
@blorente blorente MACVENTURE: Fix small bug in save reading d7d03ba
@blorente blorente MACVENTURE: Small fixed 0fc3e90
@blorente blorente MACVENTURE: Add attribute set function e8725ae
@blorente blorente MACVENTURE: Add untested script engine 9b052d0
@blorente blorente MACVENTURE: Add opcodes for script engine 96f9910
@blorente blorente MACVENTURE: Begin implementing object queue 60d5ef5
@blorente blorente MACVENTURE: Fix some minor warnings 2fbff0e
@blorente blorente MACVENTURE: Add enqueue text bbf0c62
@blorente blorente MACVENTURE: Add PPIC0, 1 and 2 decoding ccc76f2
@blorente blorente MACVENTURE: Test PPIC1 and 2 decoding 1cee6ca
@blorente blorente MACVENTURE: Add graphics blitting 15de1a2
@blorente blorente MACVENTURE: Add & test PPIC3 Huffman loading 27ecdea
@blorente blorente MACVENTURE: Complete ppic blitting 4d8f8fd
@blorente blorente MACVENTURE: Implemente dynamic object drawing 0743e95
@blorente blorente MACVENTURE: Fix dymanic object drawing 1540674
@blorente blorente MACVENTURE: Hack for dynamic object drawing 0fb344d
@blorente blorente MACVENTURE: Game window object selection and some more opcodes ec7eb7c
@blorente blorente MACVENTURE: Major push in functionality and rendering ba5ed7f
@blorente blorente MACVENTURE: Script engine fixes 499ebc0
@blorente blorente MACVENTURE: Complete text decoding 8dd52b6
@blorente blorente MACVENTURE: Add a small hack for decoding 25f086e
@blorente blorente MACVENTURE: Add rect collission for main game window 4837b77
@blorente blorente MACVENTURE: Fix rect collission 45a2aa9
@blorente blorente MACVENTURE: Add inventory callback 8bee2a7
@blorente blorente MACVENTURE: Fix small script bug 764d0ad
@blorente blorente MACVENTURE: Add first drag implementation 08588eb
@blorente blorente MACVENTURE: Minor fixes and skull rising 0485483
@blorente blorente MACVENTURE: Add proper capitalization 680790a
@blorente blorente MACVENTURE: Fix minor object drawing bug 246fec2
@blorente blorente MACVENTURE: Add scene transition 517acee
@blorente blorente MACVENTURE: Fix game detection 03a9ad4
@blorente blorente MACVENTURE: Fix clicks and dragging offset 9403ef7
@blorente blorente MACVENTURE: Fix object selection fallthrough a6e1202
@blorente blorente MACVENTURE: Add double click support 5417022
@blorente blorente MACVENTURE: Add initial text rendering 46a85f0
@blorente blorente MACVENTURE: Add double click b460964
@blorente blorente MACVENTURE: Tidy up Inventory window system 9905cd2
@blorente blorente MACVENTURE: First version of working drag 528283f
@blorente blorente MACVENTURE: Fix error with second scene 87540ea
@blorente blorente MACVENTURE: Fix object selection in inventory 6da240a
@blorente blorente MACVENTURE: Adjust drag selection to account for window movement 4662642
@blorente blorente MACVENTURE: Fix inventory 8aec5e1
@blorente blorente MACVENTURE: Fix temporal window reallocation b35ef40
@blorente blorente MACVENTURE: Fix the removal of objects in window 9ac1253
@blorente blorente MACVENTURE: Fix exits drawing fdd949b
@blorente blorente MACVENTURE: Fix click-through and refactor b6acfe8
@blorente blorente MACVENTURE: Begin fixing second inventory problem a5a094b
@blorente blorente MACVENTURE: Fix torch drawing problem 19d7321
@blorente blorente MACVENUTRE: Fix PPIC0 Drawing bc29c37
@blorente blorente MACVENTURE: Fix drag screen overflow problem d826fcb
@blorente blorente MACVENTURE: Fix dragged object move recognition 31a3296
@blorente blorente MACVENTURE: Correct object drop position 777923b
@blorente blorente MACVENTURE: FIx pesky random drag selection 2ba8b1c
@blorente blorente MACVENTURE: Soften conditions for object selection c36f8c4
@blorente blorente MACVENTURE: Fix inventory window selection f8efc58
@blorente blorente MACVENTURE: Refactor asset load checking e32c126
@blorente blorente MACVENTURE: Remove unnecesary window code dad1edc
@blorente blorente MACVENTURE: Add text input logic 8162483
@blorente blorente MACVENTURE: Add dialog system 2a521bb
@eriktorbjorn eriktorbjorn commented on the diff Aug 16, 2016
engines/macventure/macventure.cpp
+ _gameChanged = true;
+}
+
+void MacVentureEngine::winGame() {
+ _gui->showPrebuiltDialog(kWinGameDialog);
+ _gameState = kGameStateWinnig;
+}
+
+void MacVentureEngine::loseGame() {
+ _gui->showPrebuiltDialog(kLoseGameDialog);
+ _paused = true;
+ //_gameState = kGameStateLosing;
+}
+
+void MacVentureEngine::clickToContinue() {
+ _clickToContinue = true;
@eriktorbjorn
eriktorbjorn Aug 16, 2016 Member

This appears to get called once before _clickToContinue has been initialized.

@blorente
blorente Aug 16, 2016 edited Contributor

I can't see it in the flow of the program, but in any case this function can and should be used to initialize it, since the engine starts requesting a click to continue.

Is it giving any trouble?

@eriktorbjorn
eriktorbjorn Aug 16, 2016 Member

Sorry, I put this comment on the wrong line. I meant the needsClickToContinue() function. And that warning no longer seems to happen.

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/macventure.cpp
+void MacVentureEngine::enqueueSound(SoundQueueID type, ObjID target) {
+ QueuedSound newSound;
+ newSound.id = type;
+ newSound.reference = target;
+ _soundQueue.push_back(newSound);
+}
+
+void MacVentureEngine::handleObjectSelect(ObjID objID, WindowReference win, bool shiftPressed, bool isDoubleClick) {
+ if (win == kExitsWindow) {
+ win = kMainGameWindow;
+ }
+
+ const WindowData &windata = _gui->getWindowData(win);
+
+ if (shiftPressed) {
+ // Do shift ;)
@sev-
sev- Aug 16, 2016 Member

Is it a TODO?

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/macventure.cpp
+ gameChanged();
+ break;
+ case kTextNewLine:
+ _gui->printText(Common::String(""));
+ gameChanged();
+ break;
+ case kTextPlain:
+ _gui->printText(_world->getText(text.asset, text.source, text.destination));
+ gameChanged();
+ break;
+ }
+ }
+}
+
+void MacVentureEngine::playSounds(bool pause) {
+ int delay=0;
@sev-
sev- Aug 16, 2016 Member

Bad code formatting

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/macventure.cpp
+ _currentSelection.clear();
+ _destObject = 0;
+ setDeltaPoint(Common::Point(0, 0));
+ _cmdReady = false;
+}
+
+void MacVentureEngine::unselectAll() {
+ while (!_currentSelection.empty()) {
+ unselectObject(_currentSelection.front());
+ }
+}
+
+void MacVentureEngine::selectObject(ObjID objID) {
+ if (!_currentSelection.empty()) {
+ if (findParentWindow(objID) != findParentWindow(_currentSelection[0])) {
+ //unselectAll();
@sev-
sev- Aug 16, 2016 Member

I recommend to put a TODO here, as it will be easier to search for not finished places. Same for the other similar occurences.

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/macventure.cpp
+
+void MacVentureEngine::selectObject(ObjID objID) {
+ if (!_currentSelection.empty()) {
+ if (findParentWindow(objID) != findParentWindow(_currentSelection[0])) {
+ //unselectAll();
+ }
+ }
+ if (findObjectInArray(objID, _currentSelection) == -1) {
+ _currentSelection.push_back(objID);
+ highlightExit(objID);
+ }
+}
+
+void MacVentureEngine::unselectObject(ObjID objID) {
+ int idxCur = findObjectInArray(objID, _currentSelection);
+ if (idxCur != -1){
@sev-
sev- Aug 16, 2016 Member

Bad formatting. No space in front of the opening bracket.

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/macventure.cpp
+void MacVentureEngine::updateExits() {
+ _gui->clearExits();
+ _gui->unselectExits();
+
+ Common::Array<ObjID> exits = _world->getChildren(_world->getObjAttr(1, kAttrParentObject), true);
+ for (uint i = 0; i < exits.size(); i++)
+ _gui->updateExit(exits[i]);
+
+}
+
+int MacVentureEngine::findObjectInArray(ObjID objID, const Common::Array<ObjID> &list) {
+ // Find the object in the current selection
+ bool found = false;
+ uint i = 0;
+ while (i < list.size() && !found) {
+ if (list[i] == objID) found = true;
@sev-
sev- Aug 16, 2016 Member

You are using way too many one line statements. Please consider rearranging the code.

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/macventure.cpp
+ }
+ // HACK, should use iterator
+ return found ? i : -1;
+}
+
+uint MacVentureEngine::getPrefixNdx(ObjID obj) {
+ return _world->getObjAttr(obj, kAttrPrefixes);
+}
+
+Common::String MacVentureEngine::getPrefixString(uint flag, ObjID obj) {
+ uint ndx = getPrefixNdx(obj);
+ ndx = ((ndx) >> flag) & 3;
+ if (ndx) {
+ return _decodingNamingArticles->getString(ndx);
+ } else {
+ return Common::String("m1551gn0 ");
@sev-
sev- Aug 16, 2016 Member

What is this magic string?

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/macventure.cpp
+Common::String MacVentureEngine::getPrefixString(uint flag, ObjID obj) {
+ uint ndx = getPrefixNdx(obj);
+ ndx = ((ndx) >> flag) & 3;
+ if (ndx) {
+ return _decodingNamingArticles->getString(ndx);
+ } else {
+ return Common::String("m1551gn0 ");
+ }
+}
+
+Common::String MacVentureEngine::getNoun(ObjID ndx) {
+ return _decodingIndirectArticles->getString(ndx);
+}
+
+void MacVentureEngine::highlightExit(ObjID objID) {
+ //ObjID ctl = _gui->getWinChild(obj);
@sev-
sev- Aug 16, 2016 edited Member

Same thing. Put a TODO, or output a warning("STUB: highlightExit");

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/macventure.cpp
+ Common::Point p(_world->getObjAttr(objID, kAttrPosX), _world->getObjAttr(objID, kAttrPosY));
+ WindowReference invID = _gui->createInventoryWindow(objID);
+ _gui->setWindowTitle(invID, _world->getText(objID, objID, objID));
+ _gui->updateWindowInfo(invID, objID, _world->getChildren(objID, true));
+ _gui->updateWindow(invID, _world->getObjAttr(objID, kAttrContainerOpen));
+ }
+}
+
+void MacVentureEngine::closeObject(ObjID objID) {
+ warning("closeObject: not fully implemented");
+ _gui->tryCloseWindow(getObjWindow(objID));
+ return;
+}
+
+void MacVentureEngine::checkObject(QueuedObject old) {
+ //warning("checkObject: unimplemented");
@sev-
sev- Aug 16, 2016 Member

Is this still unimplemented or it is just a leftover?

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/macventure.cpp
+ old.invisible != _world->getObjAttr(id, kAttrUnclickable)) {
+ updateWindow(findParentWindow(id));
+ }
+
+ if (_world->getObjAttr(id, kAttrIsExit)) {
+ if (hasChanged ||
+ old.hidden != _world->getObjAttr(id, kAttrHiddenExit) ||
+ old.exitx != _world->getObjAttr(id, kAttrExitX) ||
+ old.exity != _world->getObjAttr(id, kAttrExitY))
+ _gui->updateExit(id);
+ }
+ WindowReference win = getObjWindow(id);
+ ObjID cur = id;
+ ObjID root = _world->getObjAttr(1, kAttrParentObject);
+ while (cur != root) {
+ if (cur == 0 || !_world->getObjAttr(cur, kAttrContainerOpen)) break;
@sev-
sev- Aug 16, 2016 Member

oneliners, oneliners.

@sev- sev- commented on the diff Aug 16, 2016
engines/macventure/macventure.cpp
+ return kNoCommand;
+ }
+}
+
+// Data retrieval
+
+bool MacVentureEngine::isPaused() {
+ return _paused;
+}
+
+bool MacVentureEngine::needsClickToContinue() {
+ return _clickToContinue;
+}
+
+Common::String MacVentureEngine::getCommandsPausedString() const {
+ return Common::String("Click to continue");
@sev-
sev- Aug 16, 2016 Member

Should this perhaps be translatable?

@blorente
blorente Aug 19, 2016 Contributor

I think there are no versions of the games in other languages, so unless we were to create the assets ourselves, I don't think it could be played in other languages.

@sev- sev- and 1 other commented on an outdated diff Aug 16, 2016
engines/macventure/macventure.cpp
+// Data retrieval
+
+bool MacVentureEngine::isPaused() {
+ return _paused;
+}
+
+bool MacVentureEngine::needsClickToContinue() {
+ return _clickToContinue;
+}
+
+Common::String MacVentureEngine::getCommandsPausedString() const {
+ return Common::String("Click to continue");
+}
+
+Common::String MacVentureEngine::getFilePath(FilePathID id) const {
+ const Common::Array<Common::String> &names = _filenames->getStrings();
@sev-
sev- Aug 16, 2016 Member

It doesn't look to me like an effective solution unless you're calling it very rarely. E.g. why are you recreating the whole array on every call when you need only one value?

@blorente
blorente Aug 17, 2016 Contributor

It is called very rarely, only once per data file at startup (about 5-6 times total), but you are right, it should be fixed.

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/macventure.cpp
+}
+
+Common::String MacVentureEngine::getFilePath(FilePathID id) const {
+ const Common::Array<Common::String> &names = _filenames->getStrings();
+ if (id <= 3) { // We don't want a file in the subdirectory
+ return Common::String(names[id]);
+ } else { // We want a game file
+ return Common::String(names[3] + "/" + names[id]);
+ }
+}
+
+bool MacVentureEngine::isOldText() const {
+ return _oldTextEncoding;
+}
+
+const HuffmanLists * MacVentureEngine::getDecodingHuffman() const {
@sev-
sev- Aug 16, 2016 Member

Bad code formatting. Stick '*' to MacVentureEngine

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/macventure.cpp
+}
+
+bool MacVentureEngine::isOldText() const {
+ return _oldTextEncoding;
+}
+
+const HuffmanLists * MacVentureEngine::getDecodingHuffman() const {
+ return _textHuffman;
+}
+
+uint32 MacVentureEngine::randBetween(uint32 min, uint32 max) {
+ return _rnd->getRandomNumber(max - min) + min;
+}
+
+uint32 MacVentureEngine::getInvolvedObjects() {
+ return (_selectedControl ? getGlobalSettings()._cmdArgCnts[_selectedControl - 1] : 3000);
@sev-
sev- Aug 16, 2016 Member

What is this magic number 3000?

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/macventure.h
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ */
+
+#ifndef MACVENTURE_H
@sev-
sev- Aug 16, 2016 Member

It should be MACVENTURE_MACVENTURE_H

@sev- sev- and 1 other commented on an outdated diff Aug 16, 2016
engines/macventure/menu.cpp
+
+namespace MacVenture {
+
+Menu::Menu(MacVentureEngine *engine, Graphics::MacWindowManager *wm) {
+
+ _engine = engine;
+ _wm = wm;
+
+ _menu = _wm->addMenu();
+
+ if (!loadMenuData())
+ error("Could not load menu data from %s", _engine->getGameFileName());
+}
+
+Menu::~Menu() {
+ delete _
@blorente
blorente Aug 16, 2016 Contributor

That file has already been deleted, sorry for not noticing earlier.

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/prebuilt_dialogs.cpp
+* GNU General Public License for more details.
+
+* You should have received a copy of the GNU General Public License
+* along with this program; if not, write to the Free Software
+* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+*
+*/
+
+#include "macventure/prebuilt_dialogs.h"
+
+namespace MacVenture {
+
+PrebuiltDialog prebuiltDialogs[kPrebuiltDialogCount] = {
+
+ {/* kSaveAsDialog */
+ Common::Rect(0, 146, 456, 254),
@sev-
sev- Aug 16, 2016 Member

..and Common::Point/Common::Rect, as that will require global constructors.

@sev- sev- commented on the diff Aug 16, 2016
engines/macventure/saveload.cpp
+ Common::InSaveFile *file;
+ if(!(file = getSaveFileManager()->openForLoading(saveFileName))) {
+ error("missing savegame file %s", saveFileName.c_str());
+ }
+ _world->loadGameFrom(file);
+ reset();
+ return Common::kNoError;
+}
+
+Common::Error MacVentureEngine::saveGameState(int slot, const Common::String &desc) {
+ Common::String saveFileName = Common::String::format("%s.%03d", _targetName.c_str(), slot);
+ Common::SaveFileManager *manager = getSaveFileManager();
+ // HACK Get a real name!
+ Common::OutSaveFile *file = manager->openForSaving(saveFileName);
+ _world->saveGameInto(file);
+ writeMetaData(file, desc);
@sev-
sev- Aug 16, 2016 Member

Consider storing the game screenshot and playtime.

@blorente
blorente Aug 21, 2016 Contributor

It should be done now :)

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/script.cpp
+* You should have received a copy of the GNU General Public License
+* along with this program; if not, write to the Free Software
+* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+*
+*/
+
+#include "common/system.h"
+
+#include "macventure/macventure.h"
+#include "macventure/script.h"
+#include "macventure/world.h"
+#include "macventure/container.h"
+
+namespace MacVenture {
+
+ScriptEngine::ScriptEngine(MacVentureEngine * engine, World * world) {
@sev-
sev- Aug 16, 2016 Member

Bad code formatting.

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/script.cpp
+
+void ScriptEngine::reset() {
+ _frames.clear();
+}
+
+bool ScriptEngine::execFrame(bool execAll) {
+ bool doFirst = execAll;
+ bool doFamily = false;
+ bool fail;
+
+ EngineFrame *frame = &_frames.front();
+
+ // Do first dispatch script (script 0)
+ if (frame->haltedInFirst || doFirst) { // We were stuck or it's the first time
+ frame->haltedInFirst = false;
+ if (doFirst) { fail = loadScript(frame, 0); }
@sev-
sev- Aug 16, 2016 Member

This must be formatted properly.

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/script.cpp
+ if (fail) {
+ frame->haltedInFirst = true;
+ _engine->preparedToRun();
+ return true;
+ }
+ doFamily = true;
+ frame->familyIdx = 0;
+ }
+
+ // Do scripts in the family of player (ObjID 1)
+ if (frame->haltedInFamily || doFamily) { // We have to do the family or we were stuck here
+ frame->haltedInFamily = false;
+ Common::Array<ObjID> family = _world->getFamily(_world->getObjAttr(1, kAttrParentObject), false);
+ uint32 i = frame->familyIdx;
+ for (; i < family.size(); i++) {
+ if (doFamily) { fail = loadScript(frame, family[i]); }
@sev-
sev- Aug 16, 2016 Member

Same thing with formatting.

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/script.cpp
+ if (doFirst) { fail = loadScript(frame, 0); }
+ else { fail = resumeFunc(frame); }
+ if (fail) {
+ frame->haltedInFirst = true;
+ _engine->preparedToRun();
+ return true;
+ }
+ doFamily = true;
+ frame->familyIdx = 0;
+ }
+
+ // Do scripts in the family of player (ObjID 1)
+ if (frame->haltedInFamily || doFamily) { // We have to do the family or we were stuck here
+ frame->haltedInFamily = false;
+ Common::Array<ObjID> family = _world->getFamily(_world->getObjAttr(1, kAttrParentObject), false);
+ uint32 i = frame->familyIdx;
@sev-
sev- Aug 16, 2016 Member

Why i is not declared right in the for() statement. You're not using it out of that scope anyway.

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/script.cpp
+ // Halted in saves
+ if (frame->haltedInSaves) {
+ frame->haltedInSaves = false;
+ if (resumeFunc(frame)) {
+ frame->haltedInSaves = true;
+ _engine->preparedToRun();
+ return true;
+ }
+ }
+
+ int highest = 0;
+ uint localHigh = 0;
+ do { // Saved function calls
+ highest = 0;
+ for (uint i = 0; i <frame->saves.size(); i++)
+ {
@sev-
sev- Aug 16, 2016 Member

Code formatting.

@sev- sev- and 1 other commented on an outdated diff Aug 16, 2016
engines/macventure/script.cpp
+ break;
+ case 0xe6: //get fibonacci (joke)
+ ope6GFIB(state, frame);
+ break;
+ case 0xe7: //calc fibonacci
+ ope7CFIB(state, frame);
+ break;
+ default:
+ op00NOOP(op);
+ }
+ }
+ }
+ return false;
+}
+
+word ScriptEngine::neg16(word val) {
@sev-
sev- Aug 16, 2016 Member

what is word? Is it uint16?

@blorente
blorente Aug 17, 2016 Contributor

It's actually int16, the format guide didn't specify it and I typedef'd it until I knew what it was. I started with uint16, but it was dead wrong. Anyways, I personaly prefer to have it typedef'd, but of course it can be changed.

@sev-
sev- Aug 17, 2016 Member

I would suggest switching to int16 then. The thing is that most of your code is using our types, and this particular one looks a bit alien.

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/script.cpp
+word ScriptEngine::neg8(word val) {
+ if (val & 0x80)
+ val = -((val ^ 0xff) + 1);
+ return val;
+}
+
+word ScriptEngine::sumChildrenAttr(word obj, word attr, bool recursive) {
+ word sum = 0;
+ Common::Array<ObjID> children = _world->getChildren(obj, recursive);
+ for (Common::Array<ObjID>::const_iterator it = children.begin(); it != children.end(); it++) {
+ sum += _world->getObjAttr(*it, attr);
+ }
+ return sum;
+}
+
+void MacVenture::ScriptEngine::op80GATT(EngineState * state, EngineFrame * frame) {
@sev-
sev- Aug 16, 2016 Member

Formatting is bad in all these prototypes.

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/script.cpp
+ word a = state->pop();
+ word b = state->pop();
+ word c = state->pop();
+ state->push(a);
+ state->push(c);
+ state->push(b);
+}
+
+void ScriptEngine::op95SORT(EngineState * state, EngineFrame * frame) {
+ word step = neg16(state->pop());
+ word num = neg16(state->pop());
+ step %= num;
+ if (step<0) step += num;
+ word end = 0;
+ word start = 0;
+ for (word i = 1;i<num;i++) {
@sev-
sev- Aug 16, 2016 Member

Formatting.

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/script.cpp
+}
+
+void ScriptEngine::op94SHUFF(EngineState * state, EngineFrame * frame) {
+ word a = state->pop();
+ word b = state->pop();
+ word c = state->pop();
+ state->push(a);
+ state->push(c);
+ state->push(b);
+}
+
+void ScriptEngine::op95SORT(EngineState * state, EngineFrame * frame) {
+ word step = neg16(state->pop());
+ word num = neg16(state->pop());
+ step %= num;
+ if (step<0) step += num;
@sev-
sev- Aug 16, 2016 Member

formatting, one-liner.

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/script.cpp
+ int16 b = state->pop();
+ int16 a = state->pop();
+ state->push((a / b) | 0);
+}
+
+void ScriptEngine::op9cMOD(EngineState * state, EngineFrame * frame) {
+ int16 b = state->pop();
+ int16 a = state->pop();
+ state->push(a % b);
+}
+
+void ScriptEngine::op9dDMOD(EngineState * state, EngineFrame * frame) {
+ word b = state->pop();
+ word a = state->pop();
+ state->push(a % b);
+ state->push((a / b) | 0);
@sev-
sev- Aug 16, 2016 Member

you may get division by zero here. Please add a check.

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/script.cpp
+ for (uint i = 0; i < frame->saves.size(); i++) {
+ if (frame->saves[i].func == func)
+ frame->saves[i].rank = 0;
+ }
+}
+
+void ScriptEngine::opb8CLOW(EngineState * state, EngineFrame * frame) {
+ word hi = state->pop();
+ for (uint i = 0;i<frame->saves.size();i++)
+ if (frame->saves[i].rank <= hi)
+ frame->saves[i].rank = 0;
+}
+
+void ScriptEngine::opb9CHI(EngineState * state, EngineFrame * frame) {
+ word lo = state->pop();
+ for (uint i = 0;i<frame->saves.size();i++)
@sev-
sev- Aug 16, 2016 Member

Formatting, everywhere in this file.

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/sound.cpp
+}
+
+void SoundAsset::decode1a(Common::SeekableReadStream *stream) {
+ warning("Decode sound 0x1a untested");
+ Common::Array<byte> wavtable;
+ stream->seek(0x220, SEEK_SET);
+ for (uint i = 0; i < 16; i++) {
+ wavtable.push_back(stream->readByte());
+ }
+ _length = stream->readUint32BE();
+ //Unused
+ stream->readUint16BE();
+ _frequency = (stream->readUint32BE() * 22100 / 0x10000) | 0;
+ byte ch = 0;
+ for (uint i = 0; i < _length; i++) {
+ if (i & 1) ch >>= 4;
@sev-
sev- Aug 16, 2016 Member

Oneliners.

@sev- sev- and 1 other commented on an outdated diff Aug 16, 2016
engines/macventure/sound.cpp
+ if (i & 1) ch <<= 4;
+ else ch = stream->readByte();
+ _data.push_back(wavtable[(ch >> 4) & 0xf]);
+ }
+}
+
+void SoundAsset::decode7e(Common::SeekableReadStream *stream) {
+ Common::Array<byte> wavtable;
+ stream->seek(0xc2, SEEK_SET);
+ for (uint i = 0; i < 16; i++) {
+ wavtable.push_back(stream->readByte());
+ }
+ //Unused
+ stream->readUint32BE();
+ _length = stream->readUint32BE();
+ _frequency = (stream->readUint32BE() * 22100 / 0x10000) | 0;
@sev-
sev- Aug 16, 2016 Member

What is the purpose of '| 0'?

@wjp
wjp Aug 16, 2016 Member

That's a javascript thing to turn Infinity (caused by divisions by 0) into 0 I think. They'll all need to be changed.

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/stringtable.h
+
+ bool loadStrings() {
+ Common::MacResIDArray resArray;
+ Common::SeekableReadStream *res;
+
+ if ((resArray = _resourceManager->getResIDArray(MKTAG('S', 'T', 'R', '#'))).size() == 0)
+ return false;
+
+ res = _resourceManager->getResource(MKTAG('S', 'T', 'R', '#'), _id);
+
+ _strings.push_back("dummy"); // String tables are 1-indexed
+ uint16 numStrings = res->readUint16BE();
+ uint8 strLength = 0;
+ for (uint i = 0; i < numStrings; ++i) {
+ strLength = res->readByte();
+ char* str = new char[strLength + 1];
@sev-
sev- Aug 16, 2016 Member

Formatting.

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/tests/ktest.h
@@ -0,0 +1,36 @@
+#ifndef KWORKS_TESTS_H
@sev-
sev- Aug 16, 2016 Member

KWORKS?

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/text.cpp
+ _huffman = huffman;
+ _isOld = isOld;
+ _engine = engine;
+
+ if (_isOld) {
+ decodeOld();
+ } else {
+ decodeHuffman();
+ }
+}
+
+void TextAsset::decodeOld() {
+ Common::SeekableReadStream *res = _container->getItem(_id);
+ uint16 strLen = res->readUint16BE();
+ Common::BitStream32BELSB stream(res, true);
+ char* str = new char[strLen + 1];
@sev-
sev- Aug 16, 2016 Member

Formatting

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/text.cpp
+ } else { // Ascii A-Z
+ c = val + 0x40;
+ }
+ lowercase = true;
+ } else if (val == 0x1B) {
+ if (lowercase) {
+ c = '.';
+ } else {
+ c = ',';
+ }
+ lowercase = true;
+ } else if (val == 0x1C) {
+ if (lowercase) {
+ c = 0x27; // Ascii '
+ } else {
+ c = 0x22; // Ascii '"'
@sev-
sev- Aug 16, 2016 Member

why not use '"' and ''' above?

@sev- sev- and 1 other commented on an outdated diff Aug 16, 2016
engines/macventure/text.cpp
+ c = stream.getBits(8);
+ lowercase = true;
+ } else if (val == 0x1F) {
+ lowercase = !lowercase;
+ } else {
+ warning("Unrecognized char in old text %d, pos %d", _id, i);
+ }
+ str[i] = c;
+ }
+
+ str[strLen] = '\0';
+ debugC(3, kMVDebugText, "Decoded %d string (old): %s", _id, str);
+ _decoded = Common::String(str);
+}
+
+void TextAsset::decodeHuffman() {
@sev-
sev- Aug 16, 2016 Member

Are you aware that we have a generic Huffman decoder in common/huffman.h?

@blorente
blorente Aug 17, 2016 Contributor

I am, and I tried it. It didn't work when I tried it, and I thought it was because ScummVM's decoder worked with equality (==), while this one comapred the symbols by greater-than. Although, as pointed out in another comment, it might have been that there was an invalid value being read from beyond the end of a file.

@sev- sev- commented on an outdated diff Aug 16, 2016
engines/macventure/world.cpp
+ warning("Saving the game not yet tested!");
+ // Save attibutes
+ Common::Array<AttributeGroup>::const_iterator itg;
+ for(itg = _groups.begin(); itg != _groups.end(); itg++) {
+ Common::Array<Attribute>::const_iterator ita;
+ for (ita = itg->begin(); ita != itg->end(); ita++) {
+ file->writeUint16BE((*ita));
+ }
+ }
+ // Save globals
+ Common::Array<uint16>::const_iterator global;
+ for (global = _globals.begin(); global != _globals.end(); global++) {
+ file->writeUint16BE((*global));
+ }
+ // Save text
+ _text = "Hello";
@sev-
sev- Aug 16, 2016 Member

Is that a placeholder? If so, consider marking it with TODO

blorente added some commits Aug 7, 2016
@blorente blorente MACVENTURE: Add conditionals to engine destructors dd35275
@blorente blorente MACVENTURE: Fix window resizing bug 9676b6f
@blorente blorente MACVENTURE: Refactor object drawing 4f6609f
@blorente blorente MACVENTURE: Fix console drawing bug 5601859
@blorente blorente MACVENTURE: Fix small if fd601f0
@blorente blorente MACVENTURE: Fix double free on sound 7f533ff
@blorente blorente MACVENTURE: Add win game dialog 5c38a0c
@blorente blorente MACVENTURE: Fix indentation in dialog system 90298b0
@blorente blorente MACVENTURE: Fix engine mause in dialog c8a2b0a
@blorente blorente MACVENTURE: Refactor world to extract new game method 1d5b7a6
@blorente blorente MACVENTURE: Add 'new game' functionality 0cd9e87
@blorente blorente MACVENTURE: Fix win game dialog 0213854
@blorente blorente MACVENTURE: Add lose game dialog e886f2c
@blorente blorente MACVENTURE: Add proper flags to gui debug rects 896e08c
@blorente blorente MACVENTURE: Unify NULLs 1bb3d14
@blorente blorente MACVENTURE: Minor refactorings 4e3daab
@blorente blorente MACVENTURE: Some more refactoring 580c813
@blorente blorente MACVENTURE: Delete duplicate code 7b9c63b
@blorente blorente MACVENTURE: Delete unnecessary attribute 8417e6f
@blorente blorente MACVENTURE: Implement gui reloading 479f01b
@blorente blorente MACVENTURE: Fix minor memory leaks 44a6f8a
@blorente blorente MACVENTURE: Add wrapper class for global settings 019f3d4
@blorente blorente MACVENTURE: Fix double free 2f2d9be
@blorente blorente MACVENTURE: Fix leak on loadControls 28698ba
@blorente blorente MACVENTURE: Fix mismatched new and delete[] d1cd772
@blorente blorente MACVENTURE: Fix leak on text decoding 6f9a171
@blorente blorente MACVENTURE: Fix window object drawing 2f13686
@blorente blorente MACVENTURE: Fix image overflow blitting d86a426
@blorente blorente MACVENTURE: Fix operate command 62af855
@blorente blorente MACVENTURE: Fix lost constant fd01961
@blorente blorente MACVENTURE: Fix indentation and braces 09fe00e
@blorente blorente MACVENTURE: Fix some compiler warnings 9c0777e
@blorente blorente MACVENTURE: Fix cursor warning 234a3b9
@blorente blorente MACVENTURE: Add prefixes to error messages b1eb6da
@blorente blorente MACVENTURE: Remove unused files 1c687a7
@blorente blorente MACVENTURE: Extract implementation of Container b24c047
@blorente blorente MACVENTURE: Implement quit from menu f928dee
@blorente blorente MACVENTURE: Fix debug messages 34fdec3
@blorente blorente MACVENTURE: Break up one-line ifs and fix braces e5cf033
@blorente blorente MACVENTURE: Remove unnecessary comment 8bea8ec
@blorente blorente MACVENTURE: Fix indentation fa815e7
@blorente blorente MACVENTURE: Enforce const in prebuilt dialogs ace5156
@blorente blorente MACVENTURE: Remove constructors from prebuilt dialogs 68b171f
@blorente blorente MACVENTURE: Fix formatting 19c7bcf
@blorente blorente MACVENTURE: Remove leftover comments and document magic constants d8e4d18
@blorente blorente MACVENTURE: Merge detection tables into detection.cpp 12ce17d
@blorente blorente MACVENTURE: Remove word typedef 97af2b6
@blorente blorente MACVENTURE: Remove JavaScript constructs 69f2302
@blorente blorente MACVENTURE: Fix double overflow when blitting ccd5ad5
@blorente
Contributor

I rebased to erase the commits regarding the test framework. Nothing else seems broken, but I keep a backup just in case. Please do not hesitate to comment if something is not working anymore.

@eriktorbjorn eriktorbjorn commented on an outdated diff Aug 22, 2016
engines/macventure/gui.cpp
+ Common::Array<CommandButton>::iterator it = _controlData->begin();
+ for (; it != _controlData->end(); ++it) {
+ it->unselect();
+ }
+}
+
+void Gui::initWindows() {
+ // Game Controls Window
+ _controlsWindow = _wm.addWindow(false, false, false);
+ _controlsWindow->setDimensions(getWindowData(kCommandsWindow).bounds);
+ _controlsWindow->setActive(false);
+ _controlsWindow->setCallback(commandsWindowCallback, this);
+ loadBorders(_controlsWindow, findWindowData(kCommandsWindow).type);
+
+ // Main Game Window
+ _mainGameWindow = _wm.addWindow(true, true, true);
@eriktorbjorn
eriktorbjorn Aug 22, 2016 edited Member

Should the main game window really be editable? (I'm getting a "beam" cursor in it, and this seems to be why.) Same thing with the inventory window. (The text output window probably should perhaps be editable, but unless we support selecting text and some sort of clipboard, it might as well not be.)

@sev-
Member
sev- commented Sep 3, 2016

Thanks, the work could be continued in-tree.

@sev- sev- merged commit 9d4d4f6 into scummvm:master Sep 3, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment