Difference between revisions of "Sword25/TODO"

From ScummVM :: Wiki
Jump to navigation Jump to search
m (typo)
(Added regressions section. It's more of a bug than a TODO, but it's too early to file bug reports.)
Line 4: Line 4:
subsystem=Engine|
subsystem=Engine|
}}
}}
== Regressions ==
* In locales that use decimal comma instead of decimal point, the game refuses to start with the following error message:
<source lang="text">
ERROR: Couldn't compile "@/system/cfg.lua"
/system/cfg.lua:173: malformed number near '0.5'!
</source>
The message is produced by trydecpoint() which contrary to the comment ''is'' used in ScummVM though only, it seems, if the first attempt at converting the string failed. See the read_numeral() function.
The simplest solution I can think of would be to change trydecpoint() to use ',' instead of '.', since it has presumably already tried '.'. But I don't know if that's sufficient. Perhaps there are other decimal separators that can still fit in a char? Slightly more elaborate, I guess we could try to format a known number to one decimal places and figure out the decimal separator from that.
The regression was almost certainly caused by 5f583eda0dbd09034ae44dd726b710a18d1aaec5, "SWORD25 (LUA): Removed unused non-portable locale code".


== TODOs ==
== TODOs ==

Revision as of 16:26, 19 May 2011

TODO List
Name Sword25 Engine TODO
Technical Contact(s) Sword25 Engine Team
Subsystem Engine

Regressions

  • In locales that use decimal comma instead of decimal point, the game refuses to start with the following error message:
ERROR: Couldn't compile "@/system/cfg.lua"
/system/cfg.lua:173: malformed number near '0.5'!

The message is produced by trydecpoint() which contrary to the comment is used in ScummVM though only, it seems, if the first attempt at converting the string failed. See the read_numeral() function.

The simplest solution I can think of would be to change trydecpoint() to use ',' instead of '.', since it has presumably already tried '.'. But I don't know if that's sufficient. Perhaps there are other decimal separators that can still fit in a char? Slightly more elaborate, I guess we could try to format a known number to one decimal places and figure out the decimal separator from that.

The regression was almost certainly caused by 5f583eda0dbd09034ae44dd726b710a18d1aaec5, "SWORD25 (LUA): Removed unused non-portable locale code".

TODOs

  • Fill stubs in audio code
  • Enhance package manager to allow running with extracted data just like original did
  • Graphics:
    • Problems with the exit menu:
      1. Wrong shading of 'yes/no' glyphs
      2. Background is black instead of shadowed game screen
    • Speedup Theora video playback
    • Audio sync for Theora video playback
  • Translate comments from German
  • Fix 64-bitness
  • Check if it's completable on BE machine (first few scenes work now)
  • Rename/get rid of leftover BS_* things
  • Fix tons of warnings under MinGW and probably others
  • The engine is currently somewhat slower than the original game, particularly on game startup. Some profiling should be done to identify and improve performance.
    • A major reason for slowness is that when finding a matching file list for a given file-spec, it currently scans the entire contents of the zip file index. When we correct the Zip file implementation to properly handle folders, this will speed things up, since since it will be able to traverse down to a specific folder, and only have to scan the files of that folder.
  • Fix issues due to using libpng: The engine uses libpng. This will cause problems when building with dynamic plugins, as then either the sword25 engine then would have to be linked directly against libpng (which may not be possible everywhere), or the main ScummVM executable. But in the latter case, we get issues on systems that do not allow backlinking (such as Windows). The way we overcame this for other similar cases is to add a thin wrapper API around the library and put that into the shared code base (in this case, something like graphics/png.* might be appropriate).
  • The save system of this engine currently partially bypasses the SaveFileManager API, by (abusing) the fact that the Lua engine allows creating files in arbitrary places (it exposes fopen, fread, fwrite etc.). This is used to create a 'config.lua' configuration file. This makes it non-portable.
    In addition, the filenames used for the savestates ("0.b25s") do not comply with our naming conventions for engine savestates.
    It should be possible to overcome all this, but it might require hacking the Lua engine; or we could try to replace some of the BS2.5 script functions with our own, dynamically.
  • PersistenceService::saveGame and PersistenceService::loadGame contain code to (de)compress the save data using zlib. But we already compress savegames using zlib, so now we end up compressing them twice. Unless we need to do the compression to be compatible with saves from the original, we should get rid of this in-engine compression. If we can't get rid of it, we should add code comments that explain the reasons.
  • The following commented out code used to be in kernel/scummvmwindow.cpp; I am keeping it here in case there is still something in there that needs to be handled (which I can't tell right now).
// FIXME: Special keys detected here need to be moved into the Input Engine
// Die WindowProc aller Fenster der Klasse
LRESULT CALLBACK BS_ScummVMWindow::WindowProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
{
    switch(uMsg)
    {
    case WM_PAINT:
        ValidateRect(hwnd, NULL);
        break;

    case WM_DESTROY:
        // Das Fenster wird zerstört
        PostQuitMessage(0);
        break;

    case WM_CLOSE:
        {
            BS_Window * WindowPtr = BS_Kernel::GetInstance()->GetWindow();
            if (WindowPtr) {
                WindowPtr->SetCloseWanted(true);
            }
            break;
        }

    case WM_KEYDOWN:
        {
            // Tastendrücke, die für das Inputmodul interessant sind, werden diesem gemeldet.
            BS_InputEngine * InputPtr = BS_Kernel::GetInstance()->GetInput();

            if (InputPtr)
            {
                switch (wParam)
                {
                case VK_RETURN:
                    InputPtr->ReportCommand(BS_InputEngine::KEY_COMMAND_ENTER);
                    break;

                case VK_LEFT:
                    InputPtr->ReportCommand(BS_InputEngine::KEY_COMMAND_LEFT);
                    break;

                case VK_RIGHT:
                    InputPtr->ReportCommand(BS_InputEngine::KEY_COMMAND_RIGHT);
                    break;

                case VK_HOME:
                    InputPtr->ReportCommand(BS_InputEngine::KEY_COMMAND_HOME);
                    break;

                case VK_END:
                    InputPtr->ReportCommand(BS_InputEngine::KEY_COMMAND_END);
                    break;

                case VK_BACK:
                    InputPtr->ReportCommand(BS_InputEngine::KEY_COMMAND_BACKSPACE);
                    break;

                case VK_TAB:
                    InputPtr->ReportCommand(BS_InputEngine::KEY_COMMAND_TAB);
                    break;

                case VK_INSERT:
                    InputPtr->ReportCommand(BS_InputEngine::KEY_COMMAND_INSERT);
                    break;

                case VK_DELETE:
                    InputPtr->ReportCommand(BS_InputEngine::KEY_COMMAND_DELETE);
                    break;
                }
            }
            break;
        }

    case WM_KEYUP:
    case WM_SYSKEYUP:
        // Alle Tastendrücke werden ignoriert, damit Windows per DefWindowProc() nicht darauf
        // reagieren kann und damit unerwartete Seiteneffekte auslöst.
        // Zum Beispiel würden ALT und F10 Tastendrücke das "Menü" aktivieren und somit den Message-Loop zum Stillstand bringen.
        break;

    case WM_SYSCOMMAND:
        // Verhindern, dass der Bildschirmschoner aktiviert wird, während das Spiel läuft
        if (wParam != SC_SCREENSAVE) return DefWindowProc(hwnd,uMsg,wParam,lParam);
        break;

    case WM_CHAR:
        {
            byte theChar = static_cast<byte>(wParam & 0xff);

            // Alle Zeichen, die keine Steuerzeichen sind, werden als Buchstaben dem Input-Service mitgeteilt.
            if (theChar >= 32)
            {
                BS_InputEngine * InputPtr = BS_Kernel::GetInstance()->GetInput();
                if (InputPtr) InputPtr->ReportCharacter(theChar);
            }
        }
        break;

    case WM_SETCURSOR:
        {
            // Der Systemcursor wird in der Client-Area des Fensters nicht angezeigt, jedoch in der nicht Client-Area, damit der Benutzer das Fenster wie gewohnt
            // schließen und verschieben kann.

            // Koordinaten des Cursors in der Client-Area berechnen.
            POINT pt;
            GetCursorPos(&pt);
            ScreenToClient(hwnd, &pt);

            // Feststellen, ob sich der Cursor in der Client-Area befindet.
            // Get client rect
            RECT rc;
            GetClientRect(hwnd, &rc);

            // See if cursor is in client area
            if(PtInRect(&rc, pt))
                // In der Client-Area keinen Cursor anzeigen.
                SetCursor(NULL);
            else
                // Ausserhalb der Client-Area den Cursor anzeigen.
                SetCursor(LoadCursor(NULL, IDC_ARROW));

            return TRUE;
        }
        break;

    default:
        // Um alle anderen Vorkommnisse kümmert sich Windows
        return DefWindowProc(hwnd,uMsg,wParam,lParam);
    }

    return 0;
}

Lua TODO

  • Lua exceptions require use of setjmp (not portable) or throw/catch (but we normally disable exceptions in ScummVM builds to reduce memory usage). This needs to be dealt with. E.g. we should add a configure check for setjmp, and only enable the engines on ports that support this.
  • Lua makes use of several other potentially non-portable or "dangerous" routines, that should be avoided in ScummVM code. I noticed the following (but the list may not be complete):
    • in loslib.cpp:
      • strerror
      • system (!!)
      • remove
      • rename
      • tmpname / mktemp
      • getenv
      • clock
      • gmtime
      • localtime
      • mktime
      • time
      • difftime
      • setlocale
      • exit (replace by a call to OSystem::quit, or "just" quit the engine?)
    • in liolib.cpp and lauxlib.cpp:
      • fopen, fread, fwrite, fclose, fseek, setvbuf, ... -> use our file API instead?
    • in loadlib.cpp
      • fopen / fclose
      • this contains stuff for dealing with loadable modules (which we don't really need, do we?). So we might be able to just stub it out completely.