Skip to main content

Reviewing & Polishing

 Hello! Because I think I am mostly done with the new feature implementation, this week was mostly cleaning up the tree, and getting it ready for a PR!

Last week, I mentioned some special engines which were ported a little differently, so let's start with those.

The MetaEngines in ScummVM uses game detection to detect these games and then uses MetaEngineConnect to instantiate an engine and run it. However, if we fail at the very first step, i.e detecting a game - what happens then? For this, ScummVM uses "fallbackDetection" to approximately find the closest match to the game engine that could be used.

The AGI engine was one of my first test engines because it had an additional dependency on fallback detection, which I wanted to test from the get-go. Turns out that the resources for fallbackDetection were only used by the detection features in this case, so the object file was shifted to the DETECT_OBJS variable which would eventually be linked with the executable.

However, that might not always be the case. Some engines use the resources for fallbackDetection for themselves. What this means, is that there are 2 cases:

a) Additional engine resources are needed, but the additional resources themselves have no dependencies on the engine / low amount of dependency.

b) The additional resources are engine resources heavy.

In the case of "a)", we can provide a copy of those resources to the executable and the engine plugin. This way, detection and running the engine both work well. However, in the case of "b)", it's not really relevant to the executable that we should put in a stream of objects that relate to the engine. Thus, in this case, fallback detection features should not be linked with the executable. This was the very case with Sci & Wintermute. Since they couldn't have the fallback detection features inside the executable, what I did was to still override the very same function, but instead of the original definition - I provide a "hook" to the matching engine plugin and then call the actual fallback function definition from there. This is named "fallbackDetectExtern" inside the MetaEngineConnect class, and thus the functionality to use fallback detection is dependant on the engine plugin. It looks something like this:


static const Plugin *metaEnginePlugin = EngineMan.findPlugin(getEngineId());
if (metaEnginePlugin) {
  static const Plugin *enginePlugin = PluginMan.giveEngineFromMetaEngine(metaEnginePlugin);
  if (enginePlugin) {
   return enginePlugin->get<AdvancedMetaEngineConnect>().fallbackDetectExtern(_md5Bytes, allFiles, fslist);


The second case of a different type of split was the Mohawk engine, which has custom in-game dialogs. When an engine inherits AdvancedMetaEngine, it can have a table of extra engine options that is passed along to the constructor. Thus, to maintain the overall structure of most engines, I decided to keep the custom dialogs of Mohawk separately, inside the MetaEngineConnect class. It actually would make sense to do it that way too, since game dialogs have no need to be inside the executable. 

So, these 2 sets of engines were the most different from most engine ports.

I briefly mentioned that I had some new work to do with making the detection features as a plugin themselves, as well as arranging the makefiles in a neat way, so everything stays pretty and organized. Because enabling an engine has no effect if it's detection features are included or not (i.e detection for each engine is always included), the current additions of "DETECT_OBJS" to the module of engine themselves wouldn't exactly be neat, so I thought up of splitting the detection modules entirely. So, what's happening is that each engine has a subfolder, named detection. Here, all the new headers & detection.cpp lives. We make a module file here, and it is named as "engines/engineName/detection". We add all the objects necessary for detection here.

Over in our configuration script, we write to a file named "engines.mk" which in turn checks if each engine is enabled or not, and adds the necessary modules. What I did was basically add each engine's detection module always, without the core check if the engine is enabled or not. Since the modules are split this way, it's a neat way of grouping together detection files as well as arranging the makefiles is nice. The detection modules look something like this:

MODULE := engines/adl/detection

# Detection objects
DETECT_OBJS += $(MODULE)/detection.o


In addition to that, the detection as a plugin themselves was easy to manage from this way. To implement that, I simply added a check in the configuration which added another module to engines.mk if detection features were to be enabled as dynamic. This new module looks something like this:

MODULE := detection // Only added if dynamic-detection enabled

DETECT_OBJS_DYNAMIC=$(addprefix ../,$(DETECT_OBJS)) // Since it lives outside the engines subdirectory, add "../" prefix.

MODULE_OBJS := \
	detection.o \
	$(DETECT_OBJS_DYNAMIC) // Reuse the already existing list of detection objects

# Reset detect objects, so none of them build into the executable.
DETECT_OBJS :=

PLUGIN := 1

# Include common rules
include $(srcdir)/rules.mk


After the above was working nicely, I opened up a PR and wrote a small amount of the overall description, which would make it easier for reviewers as well as engine maintainers to follow along with the new changes. I also created a PR for ResidualVM's engines! After doing all the above things, I was able to take it easy for a day or two, which was nice because it feels like it's been forever since I had a break!

Most other time this week was spent on polishing the PR and cleaning up small issues.  I have more things to share this week, but I wanted to keep it short for now. This week, I will be trying to extend support for the new changes to our "create_project" tool. It is used to generate project files to make it easier to work with, say, Visual Studio. VS doesn't support dynamic plugins, but since all detection features link statically, these new project files should have the support for the same. So, that's the main thing I'll be focusing on this week!

My U32 PR is also getting some comments/reviews, so side-by-side, I will be working on improving that too and getting it merge-ready. Okay, that's it for now! See you next week!

Thanks for reading!

Comments

Popular posts from this blog

The (official) end of an amazing journey!

 Hello! GSoC has officially ended, and I'm in the final stage - submitting my work for the entire summer to Google! When I first received the acceptance letter in May, I was very much baffled. It was exciting and a bit scary at the same time, because I have never ever worked on a project of this scale. Well, first time for everything I guess :) So, what have I been doing this past week? Last week I left off saying my U32 PR was showing some activity, and this week, @criezy helped out a lot by reviewing my U32 PR, and providing much feedback. He also helped out pretty much entirely towards the MacOS/iOS platforms, because it was quite not an area (Objective-C) I was comfortable in. It was really a huge help. Apart from that, I mostly spent the entire week looking at the PR to see what could be changed, fixing my mistakes, and making the PR ready for merge. It has improved quite a lot!  Criezy pushed an update for iOS, which led me to believe that there might be many more missing are

Touching upon all 3 tasks

Hello! So, this week I have done things that ranged across all my tasks. Recently I checked out our sister project, ResidualVM. ResidualVM's codebase is largely similar to ScummVM, but the main difference is the addition of capabilities to run 3D games. They also take regular snapshots from ScummVM to keep up-to-date. So, the work of RTL GUI which was merged in ScummVM was also present there. When I was running the application with the Hebrew language, some popups and drop-down buttons looked quite different from the normal GUI. Upon a little investigation, it looked like paddings were being set wrong for the RTL-widgets set in the theme. This was because back when I was on my RTL task, I temporarily set a padding of "2" as a placeholder to check, and then completely forgot about it as the difference was ever so subtle. I'm glad though, that I found this, and opened a PR to fix it. English, proper paddings Hebrew - Improper paddings Hebrew, mirrored and matching paddi