MMapper icon indicating copy to clipboard operation
MMapper copied to clipboard
verified publisher icon MUME

refactor storage to use QIODevice

Open nschimme opened this issue 1 month ago • 2 comments

Summary by Sourcery

Refactor map loading and saving to work with generic QIODevice-based sources and destinations instead of direct QFile/file paths.

New Features:

  • Introduce MapSource to represent map input from files or in-memory buffers for loading and merging maps.
  • Introduce MapDestination to represent map save targets, supporting both files and web-export directories.

Enhancements:

  • Update MainWindow async load/merge/save flows and storage backends to operate on QIODevice abstractions rather than raw file paths or QFile pointers.
  • Improve error reporting and status messages when opening or saving maps fails, including directory writability checks for web exports.
  • Simplify and standardize suggested filenames in save/export dialogs across different map formats.

Build:

  • Register new MapSource and MapDestination sources in the CMake build configuration.

nschimme avatar Nov 24 '25 21:11 nschimme

Reviewer's Guide

Refactors map loading and saving to operate on generic QIODevice-based MapSource/MapDestination abstractions instead of working directly with QFile/FileSaver and raw filenames, enabling in‑memory buffers and directory destinations while centralizing detection, error handling, and device management.

Sequence diagram for loading a map via MapSource and QIODevice

sequenceDiagram
    actor User
    participant MainWindow
    participant QFileDialog
    participant MapSource
    participant ProgressCounter
    participant AbstractMapStorage
    participant AsyncLoader
    participant AsyncTask
    participant FileFormatHelper
    participant QIODevice

    User ->> MainWindow: slot_open()
    MainWindow ->> QFileDialog: getOpenFileName()
    QFileDialog -->> MainWindow: fileName
    alt fileName empty
        MainWindow ->> MainWindow: showStatusShort(No filename provided)
        MainWindow -->> User: return
    else fileName provided
        MainWindow ->> MapSource: alloc(fileName, none)
        MapSource -->> MainWindow: shared_ptr_MapSource
        MainWindow ->> MainWindow: loadFile(source)
        MainWindow ->> MainWindow: tryStartNewAsync()
        MainWindow ->> MainWindow: forceNewFile()
        MainWindow ->> ProgressCounter: ctor()
        ProgressCounter -->> MainWindow: shared_ptr_ProgressCounter
        MainWindow ->> MainWindow: getLoadOrMergeMapStorage(pc, source)
        MainWindow ->> AbstractMapStorage: Data(pc, source)
        AbstractMapStorage -->> MainWindow: storage_data
        MainWindow ->> MapSource: getIODevice()
        MapSource -->> MainWindow: shared_ptr_QIODevice
        loop iterate_formats
            MainWindow ->> QIODevice: seek(0)
            MainWindow ->> FileFormatHelper: detect(*device)
            FileFormatHelper ->> QIODevice: read header
            FileFormatHelper -->> MainWindow: format_match_or_not
        end
        MainWindow -->> MainWindow: create AbstractMapStorage for detected format
        MainWindow ->> AsyncLoader: ctor(pc, this, fileName, device, storage)
        AsyncLoader -->> MainWindow: asyncLoader_instance
        MainWindow ->> AsyncTask: begin(asyncLoader_instance)
        AsyncTask -->> MainWindow: started
    end

Sequence diagram for saving a map via MapDestination and QIODevice

sequenceDiagram
    actor User
    participant MainWindow
    participant QFileDialog
    participant MapDestination
    participant ProgressCounter
    participant AbstractMapStorage
    participant AsyncSaver
    participant AsyncTask
    participant QIODevice

    User ->> MainWindow: slot_saveAs()
    MainWindow ->> QFileDialog: getSaveFileName()
    QFileDialog -->> MainWindow: fileName
    alt fileName empty
        MainWindow ->> MainWindow: showStatusShort(No filename provided)
        MainWindow -->> User: return false
    else fileName provided
        MainWindow ->> MapDestination: alloc(fileName, SaveFormatEnum_MM2)
        MapDestination -->> MainWindow: pDest
        MainWindow ->> MainWindow: saveFile(pDest, SaveModeEnum_FULL, SaveFormatEnum_MM2)
        MainWindow ->> ProgressCounter: ctor()
        ProgressCounter -->> MainWindow: shared_ptr_ProgressCounter
        MainWindow ->> MainWindow: create AbstractMapStorage::Data(pc, pDest)
        MainWindow ->> AbstractMapStorage: construct storage for format
        AbstractMapStorage -->> MainWindow: storage
        MainWindow ->> AsyncSaver: ctor(pc, this, pDest, storage, mode, format)
        AsyncSaver -->> MainWindow: asyncSaver_instance
        MainWindow ->> AsyncTask: begin(asyncSaver_instance)
        AsyncTask -->> MainWindow: started
        Note over AsyncSaver,AbstractMapStorage: async background_save
        AsyncSaver ->> AbstractMapStorage: saveData(mapData, baseMapOnly)
        AbstractMapStorage ->> QIODevice: getDevice()
        QIODevice -->> AbstractMapStorage: writable_device
        AbstractMapStorage ->> QIODevice: write map bytes
        AbstractMapStorage -->> AsyncSaver: success_flag
        AsyncSaver ->> MapDestination: finalize(success)
        MapDestination ->> QIODevice: close via FileSaver or directory
        MapDestination -->> AsyncSaver: done
        AsyncSaver ->> MainWindow: show success or error dialog
    end

Class diagram for new MapSource and MapDestination based storage abstractions

classDiagram
    class MapSource {
        -QString m_fileName
        -shared_ptr~QIODevice~ m_device
        +static shared_ptr~MapSource~ alloc(QString fileName, optional~QByteArray~ fileContent)
        +MapSource(Badge_MapSource badge, QString fileName, shared_ptr~QIODevice~ device)
        +shared_ptr~QIODevice~ getIODevice()
        +const QString & getFileName() const
    }

    class SaveModeEnum {
        <<enumeration>>
        FULL
        BASEMAP
    }

    class SaveFormatEnum {
        <<enumeration>>
        MM2
        MM2XML
        WEB
        MMP
    }

    class MapDestination {
        -QString m_fileName
        -shared_ptr~FileSaver~ m_fileSaver
        +static shared_ptr~MapDestination~ alloc(QString fileName, SaveFormatEnum format)
        +MapDestination(Badge_MapDestination badge, QString fileName, shared_ptr~FileSaver~ fileSaver)
        +bool isFileNative() const
        +bool isDirectory() const
        +const QString & getFileName() const
        +shared_ptr~QIODevice~ getIODevice() const
        +void finalize(bool success)
    }

    class AbstractMapStorage_Data {
        +shared_ptr~ProgressCounter~ progressCounter
        +shared_ptr~MapSource~ loadSource
        +shared_ptr~MapDestination~ saveDestination
        +Data(shared_ptr~ProgressCounter~ pc, shared_ptr~MapSource~ src)
        +Data(shared_ptr~ProgressCounter~ pc, shared_ptr~MapDestination~ dest)
    }

    class AbstractMapStorage {
        -AbstractMapStorage_Data m_data
        +AbstractMapStorage(AbstractMapStorage_Data data, QObject *parent)
        +bool canLoad() const
        +bool canSave() const
        +optional~RawMapLoadData~ loadData()
        +bool saveData(const MapData & mapData, bool baseMapOnly)
        +const QString & getFilename() const
        +ProgressCounter & getProgressCounter() const
        +QIODevice & getDevice() const
        <<abstract>> bool virt_canLoad() const
        <<abstract>> bool virt_canSave() const
        <<abstract>> optional~RawMapLoadData~ virt_loadData()
        <<abstract>> bool virt_saveData(const MapLoadData & mapData)
    }

    class FileSaver {
        +void open(QString fileName)
        +void close()
        +shared_ptr~QFile~ getSharedFile() const
    }

    class QIODevice {
    }

    class ProgressCounter {
    }

    MapSource ..> QIODevice : uses
    MapDestination ..> FileSaver : wraps
    MapDestination ..> QIODevice : exposes
    AbstractMapStorage_Data --> MapSource : loadSource
    AbstractMapStorage_Data --> MapDestination : saveDestination
    AbstractMapStorage o-- AbstractMapStorage_Data : has
    AbstractMapStorage ..> MapSource : load via
    AbstractMapStorage ..> MapDestination : save via
    MapDestination ..> SaveFormatEnum
    AbstractMapStorage ..> SaveModeEnum
    AbstractMapStorage ..> ProgressCounter

Class diagram for MainWindow async IO refactor using QIODevice, MapSource, and MapDestination

classDiagram
    class MainWindow {
        +bool saveFile(shared_ptr~MapDestination~ pDest, SaveModeEnum mode, SaveFormatEnum format)
        +void loadFile(shared_ptr~MapSource~ source)
        +unique_ptr~AbstractMapStorage~ getLoadOrMergeMapStorage(shared_ptr~ProgressCounter~ pc, shared_ptr~MapSource~ source)
        +void slot_open()
        +void slot_merge()
        +bool slot_save()
        +bool slot_saveAs()
        +bool slot_exportBaseMap()
        +bool slot_exportMm2xmlMap()
        +bool slot_exportWebMap()
        +bool slot_exportMmpMap()
        +void slot_reload()
    }

    class AsyncBase {
        +AsyncBase(shared_ptr~ProgressCounter~ pc)
        +void reset()
    }

    class AsyncTask {
        +void begin(unique_ptr~AsyncBase~ task)
        +void reset()
    }

    class AsyncHelper {
        <<struct>>
        +typedef shared_ptr~QIODevice~ SharedDevice
        +typedef unique_ptr~AbstractMapStorage~ UniqueStorage
        +MainWindow & mainWindow
        +QString fileName
        +SharedDevice pDevice
        +AsyncHelper(shared_ptr~ProgressCounter~ pc, MainWindow & mw, QString name, SharedDevice pd, UniqueStorage ps, QString dialogText, CancelDispositionEnum allow_cancel)
        +void reset()
    }

    class AsyncLoader {
        +AsyncLoader(shared_ptr~ProgressCounter~ pc, MainWindow & mw, QString name, AsyncHelper_SharedDevice pd, AsyncHelper_UniqueStorage ps)
        +Result work() noexcept
    }

    class AsyncMerge {
        +AsyncMerge(shared_ptr~ProgressCounter~ pc, MainWindow & mw, QString name, AsyncHelper_SharedDevice pd, AsyncHelper_UniqueStorage ps)
        +Result work() noexcept
    }

    class AsyncSaver {
        +SaveModeEnum mode
        +SaveFormatEnum format
        +shared_ptr~MapDestination~ pMapDestination
        +AsyncSaver(shared_ptr~ProgressCounter~ pc, MainWindow & mw, shared_ptr~MapDestination~ pDest, unique_ptr~AbstractMapStorage~ ps, SaveModeEnum mode, SaveFormatEnum format)
        +Result work() noexcept
        +void finish_saving(bool success)
    }

    class MapSource {
    }

    class MapDestination {
    }

    class QIODevice {
    }

    class ProgressCounter {
    }

    class AbstractMapStorage {
    }

    class CancelDispositionEnum {
        <<enumeration>>
        Forbid
        Allow
    }

    MainWindow --> AsyncTask : manages
    AsyncHelper --|> AsyncBase
    AsyncLoader --|> AsyncHelper
    AsyncMerge --|> AsyncHelper
    AsyncSaver --|> AsyncHelper

    AsyncHelper --> MainWindow : holds_reference
    AsyncHelper --> QIODevice : pDevice
    AsyncHelper --> AbstractMapStorage : pStorage

    MainWindow ..> MapSource : loadFile
    MainWindow ..> MapDestination : saveFile
    MainWindow ..> AsyncLoader
    MainWindow ..> AsyncMerge
    MainWindow ..> AsyncSaver
    MainWindow ..> ProgressCounter

    AsyncSaver --> MapDestination : pMapDestination
    AsyncSaver ..> SaveModeEnum
    AsyncSaver ..> SaveFormatEnum

    AbstractMapStorage ..> QIODevice : getDevice

    AsyncBase ..> ProgressCounter

File-Level Changes

Change Details Files
Introduce MapSource/MapDestination abstractions and wire MainWindow async load/save flows to use them instead of QFile/FileSaver and raw filenames.
  • Added MapSource class wrapping a QIODevice plus filename, with a static alloc factory that opens either a QFile or in-memory QBuffer and throws on failure.
  • Added MapDestination class representing either a file-backed destination via FileSaver or a directory (WEB export), providing a QIODevice for file formats and a finalize hook.
  • Changed MainWindow::loadFile, slot_open, slot_reload, and slot_merge to construct MapSource via MapSource::alloc and pass shared MapSource into async helpers and storage factories.
  • Changed MainWindow::saveFile and all save/export slots to construct MapDestination via MapDestination::alloc and pass it into AsyncSaver and storage constructors, replacing direct FileSaver usage and raw filename plumbing.
  • Updated AsyncHelper/AsyncLoader/AsyncMerge/AsyncSaver to hold shared QIODevice/MapDestination instead of shared QFile/FileSaver, and to call MapDestination::finalize on completion.
 src/mapstorage/MapSource.h
src/mapstorage/MapSource.cpp
src/mapstorage/MapDestination.h
src/mapstorage/MapDestination.cpp
src/mainwindow/mainwindow-async.cpp
src/mainwindow/mainwindow-saveslots.cpp
src/mainwindow/mainwindow.cpp
Refactor AbstractMapStorage and concrete storages to operate on QIODevice via MapSource/MapDestination, decoupling them from QFile specifics.
  • Redesigned AbstractMapStorage::Data to hold either a MapSource (for loading) or MapDestination (for saving), with validation in constructors.
  • Replaced getFile() with getDevice() and changed getFilename() to delegate to the underlying MapSource/MapDestination.
  • Updated MapStorage, XmlMapStorage, PandoraMapStorage, and MmpMapStorage to use getDevice()/getFilename() for QDataStream/QXmlStreamReader/QXmlStreamWriter and metadata like size, writability, and closing.
  • Adjusted PandoraMapStorage to close the underlying QIODevice instead of QFile directly.
 src/mapstorage/abstractmapstorage.h
src/mapstorage/abstractmapstorage.cpp
src/mapstorage/mapstorage.cpp
src/mapstorage/mapstorage.h
src/mapstorage/XmlMapStorage.cpp
src/mapstorage/PandoraMapStorage.cpp
src/mapstorage/MmpMapStorage.cpp
Make file format detection and MM2 version probing work on QIODevice rather than filenames, ensuring reusable streams and correct rewinding.
  • Changed getMM2FileVersion signature to accept a QIODevice&, removing internal QFile opening logic.
  • Updated format detection helpers (detectMm2Binary, detectMm2Xml, detectPandora) and FileFormatHelper to take QIODevice&, ensuring they seek back to the beginning after probing and check seek failure in getLoadOrMergeMapStorage.
  • Ensured MapSource-provided QIODevice is rewound before each detection attempt and exception is thrown on seek failure.
 src/mapstorage/mapstorage.h
src/mapstorage/mapstorage.cpp
src/mainwindow/mainwindow-async.cpp
Improve user-facing error handling and messaging for load and merge operations while removing now-redundant helpers.
  • Removed MainWindow::chooseLoadOrMergeFileName, reportOpenFileFailure, and reportOpenFileException, inlining QFileDialog usage and error messages around MapSource::alloc/loadFile and mergeFile lambdas.
  • Standardized error handling in slot_open, slot_reload, slot_merge, and loadFile to catch std::runtime_error and unknown exceptions, showing consistent translated warning messages with filename and reason.
  • Adjusted status messages for missing directory vs filename in WEB export and other flows.
 src/mainwindow/mainwindow.cpp
src/mainwindow/mainwindow-async.cpp
Adjust save dialog behavior and suggested filenames to use baseName/suffix logic instead of regex replacements and to align with new MapDestination usage.
  • Changed suggested names for Save As, Export Base Map, Export MM2XML, and Export MMP to use QFileInfo::baseName() with appropriate suffixes (-copy.mm2, -import.mm2, -base.mm2, .xml, -mmp.xml).
  • Updated save/export slots to construct MapDestination only after the user selects a path, with consistent error handling if destination setup fails.
  • Clarified status messages when no filename or directory is provided in dialogs.
 src/mainwindow/mainwindow-saveslots.cpp
Miscellaneous cleanup related to the refactor.
  • Removed now-unused includes and types (e.g., FileSaver in mainwindow-async, various unused headers).
  • Replaced AsyncHelper::SharedFile with SharedDevice to generalize async code over QIODevice.
  • Registered new MapSource/MapDestination sources in the CMake build configuration.
 src/mainwindow/mainwindow-async.cpp
src/mainwindow/mainwindow.h
src/CMakeLists.txt
Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an issue from a review comment by replying to it. You can also reply to a review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull request title to generate a title at any time. You can also comment @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in the pull request body to generate a PR summary at any time exactly where you want it. You can also comment @sourcery-ai summary on the pull request to (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the pull request to resolve all Sourcery comments. Useful if you've already addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull request to dismiss all existing Sourcery reviews. Especially useful if you want to start fresh with a new review - don't forget to comment @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

  • Contact our support team for questions or feedback.
  • Visit our documentation for detailed guides and information.
  • Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai[bot] avatar Nov 24 '25 21:11 sourcery-ai[bot]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 66.37%. Comparing base (26d1a9d) to head (8a652df). :warning: Report is 124 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #411      +/-   ##
==========================================
- Coverage   66.48%   66.37%   -0.12%     
==========================================
  Files          85       86       +1     
  Lines        4186     3937     -249     
  Branches      255      257       +2     
==========================================
- Hits         2783     2613     -170     
+ Misses       1403     1324      -79

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Nov 24 '25 22:11 codecov[bot]