Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
#5905: Emit a warning when DR fails to locate the info file on map load
  • Loading branch information
codereader committed Apr 3, 2022
1 parent a5cd7b3 commit e0e03b2
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 13 deletions.
13 changes: 7 additions & 6 deletions radiantcore/map/Map.cpp
Expand Up @@ -966,7 +966,8 @@ void Map::saveAutomaticMapBackup(const cmd::ArgumentList& args)
void Map::registerCommands()
{
GlobalCommandSystem().addCommand("NewMap", Map::newMap);
GlobalCommandSystem().addCommand("OpenMap", Map::openMap, { cmd::ARGTYPE_STRING | cmd::ARGTYPE_OPTIONAL });
GlobalCommandSystem().addCommand("OpenMap", std::bind(&Map::openMapCmd, this, std::placeholders::_1),
{ cmd::ARGTYPE_STRING | cmd::ARGTYPE_OPTIONAL });
GlobalCommandSystem().addCommand("OpenMapFromArchive", Map::openMapFromArchive, { cmd::ARGTYPE_STRING, cmd::ARGTYPE_STRING });
GlobalCommandSystem().addCommand("ImportMap", Map::importMap);
GlobalCommandSystem().addCommand("StartMergeOperation", std::bind(&Map::startMergeOperationCmd, this, std::placeholders::_1),
Expand Down Expand Up @@ -1033,9 +1034,9 @@ void Map::newMap(const cmd::ArgumentList& args)
}
}

void Map::openMap(const cmd::ArgumentList& args)
void Map::openMapCmd(const cmd::ArgumentList& args)
{
if (!GlobalMap().askForSave(_("Open Map"))) return;
if (!askForSave(_("Open Map"))) return;

std::string candidate;

Expand All @@ -1046,7 +1047,7 @@ void Map::openMap(const cmd::ArgumentList& args)
else
{
// No arguments passed, get the map file name to load
MapFileSelection fileInfo = MapFileManager::getMapFileSelection(true, _("Open map"), filetype::TYPE_MAP);
auto fileInfo = MapFileManager::getMapFileSelection(true, _("Open map"), filetype::TYPE_MAP);
candidate = fileInfo.fullPath;
}

Expand Down Expand Up @@ -1085,8 +1086,8 @@ void Map::openMap(const cmd::ArgumentList& args)
{
GlobalMRU().insert(mapToLoad);

GlobalMap().freeMap();
GlobalMap().load(mapToLoad);
freeMap();
load(mapToLoad);
}
}

Expand Down
4 changes: 2 additions & 2 deletions radiantcore/map/Map.h
Expand Up @@ -219,10 +219,10 @@ class Map :
*/
void registerCommands();

// Static command targets for connection to the EventManager
// Command targets for connection to the EventManager
static void exportSelection(const cmd::ArgumentList& args);
static void newMap(const cmd::ArgumentList& args);
static void openMap(const cmd::ArgumentList& args);
void openMapCmd(const cmd::ArgumentList& args);
static void openMapFromArchive(const cmd::ArgumentList& args);
static void importMap(const cmd::ArgumentList& args);
void saveMapCmd(const cmd::ArgumentList& args);
Expand Down
14 changes: 9 additions & 5 deletions radiantcore/map/MapResource.cpp
Expand Up @@ -32,6 +32,7 @@
#include "algorithm/Import.h"
#include "infofile/InfoFileExporter.h"
#include "messages/MapFileOperation.h"
#include "messages/NotificationMessage.h"
#include "NodeCounter.h"
#include "MapResourceLoader.h"

Expand Down Expand Up @@ -354,17 +355,20 @@ stream::MapResourceStream::Ptr MapResource::openMapfileStream()

stream::MapResourceStream::Ptr MapResource::openInfofileStream()
{
auto fullPath = getAbsoluteResourcePath();
auto infoFilePath = os::replaceExtension(fullPath, game::current::getInfoFileExtension());

try
{
auto fullpath = getAbsoluteResourcePath();
auto infoFilename = fullpath.substr(0, fullpath.rfind('.'));
infoFilename += game::current::getInfoFileExtension();

return openFileStream(infoFilename);
return openFileStream(infoFilePath);
}
catch (const OperationException& ex)
{
// Info file load file does not stop us, just issue a warning
radiant::NotificationMessage::SendWarning(
fmt::format(_("No existing file named {0} found, could not load any group or layer information. "
"A new info file will be created the next time the map is saved."), os::getFilename(infoFilePath)),
_("Missing .darkradiant File"));
rWarning() << ex.what() << std::endl;
return stream::MapResourceStream::Ptr();
}
Expand Down
52 changes: 52 additions & 0 deletions test/MapSavingLoading.cpp
Expand Up @@ -16,6 +16,7 @@
#include "messages/FileOverwriteConfirmation.h"
#include "messages/MapFileOperation.h"
#include "messages/FileSaveConfirmation.h"
#include "messages/NotificationMessage.h"
#include "algorithm/Scene.h"
#include "algorithm/XmlUtils.h"
#include "algorithm/Primitives.h"
Expand Down Expand Up @@ -1605,4 +1606,55 @@ TEST_F(MapSavingTest, RadiantShutdownCancelWithUnsavedChanges)
});
}

// Listens for a warning message when loading maps without info file
class WarningReceiver final
{
private:
std::size_t _msgSubscription;
std::string _receivedMessage;

public:
WarningReceiver()
{
// Subscribe to the event asking for the target path
_msgSubscription = GlobalRadiantCore().getMessageBus().addListener(
radiant::IMessage::Type::Notification,
radiant::TypeListener<radiant::NotificationMessage>(
[this](radiant::NotificationMessage& msg)
{
_receivedMessage = msg.getMessage();
}));
}

std::string getReceivedMessage() const
{
return _receivedMessage;
}

~WarningReceiver()
{
GlobalRadiantCore().getMessageBus().removeListener(_msgSubscription);
}
};

TEST_F(MapLoadingTest, WarningAboutMissingInfoFile)
{
auto mapPath = createMapCopyInTempDataPath("altar.map", "altar_without_info_file.map");

// Remove the info file
auto infoFilePath = mapPath;
infoFilePath.replace_extension("darkradiant");
fs::remove(infoFilePath);

// Listen for a warning message
WarningReceiver receiver;

GlobalCommandSystem().executeCommand("OpenMap", mapPath.string());

// The name of the .darkradiant file should appear in the message
auto infoFilename = infoFilePath.filename().string();
EXPECT_NE(receiver.getReceivedMessage().find(infoFilename), std::string::npos)
<< "Didn't receive a warning about the missing .darkradiant file";
}

}

0 comments on commit e0e03b2

Please sign in to comment.