Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Export plugin for Remixed Dungeon #4158

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Mikhael-Danilov
Copy link

@Mikhael-Danilov Mikhael-Danilov changed the title Export plugin for Remixed Dungeon WiP: Export plugin for Remixed Dungeon Jan 26, 2025
@Mikhael-Danilov Mikhael-Danilov changed the title WiP: Export plugin for Remixed Dungeon Export plugin for Remixed Dungeon Jan 26, 2025
@Mikhael-Danilov
Copy link
Author

@bjorn please take a look at this.

Actual changes:

* Make sure 'parent' is forwarded to superclass
* Take 'WriteMinimized' option into account
* Fixed some translatable strings with parameters
Copy link
Member

@bjorn bjorn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bjorn please take a look at this.

Sure, I'm sorry I didn't have a look sooner! I've fixed up the code style and left some comments here.

Overall looks like a fine addition. Is the idea to let the community add maps to Remixed Dungeon?

It seems the plugin is lacking some error checking. It should at least not crash when used with unexpected data, but you might also want to add some helpful error messages (check out yyplugin.cpp for example, which uses Tiled::ERROR and Tiled::WARNING to add messages to the "Issues" view).

Don't forget to also edit dist/win/installer.wxs to add the rpd.dll to the list of plugins included in the installer.

@@ -0,0 +1 @@
{ "defaultEnable": true }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand, this plugin saves files with .json extension? This may conflict with the existing JSON export, so we can't enable this plugin by default.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit unfortunate that we need yet another copy of this JSON writing code. The only real reason this thing still exists is because arrays of numbers are stored incredibly inefficiently by Qt due to putting each value on its own (indented) line. And we're writing huge arrays of numbers.

We can leave this as-is now, but eventually I'd like to move plugins/yy/jsonwriter.cpp to libtiled, so that it can be used by all plugins. Since this writer can produce a bit nicer output (though currently its formatting is somewhat specific to GameMaker).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Source files are missing a copyright header, probably should look like:

/*
 * Remixed Dungeon Tiled Plugin
 * Copyright 2025, Mikhael Danilov <[email protected]>
 *
 * This file is part of Tiled.
 *
 * This program is free software; you can redistribute it and/or 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, see <http://www.gnu.org/licenses/>.
 */

src/plugins/rpd/rpdplugin.cpp Outdated Show resolved Hide resolved

for (int i = 0; i < layer->map()->width(); ++i) {
for (int j = 0; j < layer->map()->height(); ++j) {
int tileId = layer->asTileLayer()->cellAt(i, j).tileId();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plugin will crash if exporting a map with a layer named "logic" that isn't a tile layer. Layer type needs to be checked along with the name above (same problem with other layers below).

src/plugins/rpd/rpdplugin.cpp Outdated Show resolved Hide resolved
if (layer->name() == "objects") {
QMap<QString, QJsonArray> objects;

for (auto object : layer->asObjectGroup()->objects()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better verify layer->isObjectGroup() in the check above.


public:
enum SubFormat {
Rpd
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mSubFormat is actually unused so this can be removed.

};


class RPDSHARED_EXPORT RpdTilesetFormat : public Tiled::TilesetFormat
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can derive from Tiled::WritableTilesetFormat instead so you don't have to implement those dummy read and supportsFile overrides.

@@ -0,0 +1,86 @@
#ifndef RPDPLUGIN_H
#define RPDPLUGIN_H
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm no longer using include guards in this project, please just write #pragma once.

}

Tiled::MapToVariantConverter converter;
QVariant variant = converter.toVariant(tileset, QFileInfo(fileName).dir());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like just the same thing as JsonTilesetFormat. Why do you need it again here?

@Mikhael-Danilov Mikhael-Danilov marked this pull request as draft February 13, 2025 14:14
@Mikhael-Danilov
Copy link
Author

Thank you for the thorough review and valuable feedback!

Yes, the primary goal of this plugin is to enable the community to create and integrate custom maps for their mods in Remixed Dungeon. The game supports a modding system that allows users to create and install mods, which can include assets like maps, sprites, configuration files, and some lua scripts.

I appreciate your suggestions regarding error handling and user feedback.

I’ll work on these changes and provide an update once they’re implemented.

Mikhael-Danilov and others added 2 commits February 15, 2025 19:15
lower case short name

Co-authored-by: Thorbjørn Lindeijer <[email protected]>
simpler code

Co-authored-by: Thorbjørn Lindeijer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants