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.

Sign up for GitHub

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

Jump to bottom

Fix LDtk tile map resources export with the fast loading #5951

Merged
merged 6 commits into from
Nov 28, 2023
Merged

Fix LDtk tile map resources export with the fast loading #5951

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b5bb407
Fix LDtk tile map export with the fast loading
D8H Nov 24, 2023
75e97e2
Pass the ResourcesManager in workers constructors.
D8H Nov 27, 2023
877693d
Use a pointer.
D8H Nov 27, 2023
59e760e
Fix some tests.
D8H Nov 27, 2023
4e9a901
Try to reduce side effects.
D8H Nov 27, 2023
9770dae
Fix tests.
D8H Nov 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 13 additions & 25 deletions Core/GDCore/IDE/Project/ArbitraryResourceWorker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,10 @@ void ArbitraryResourceWorker::ExposeBitmapFont(gd::String& bitmapFontName){
};

void ArbitraryResourceWorker::ExposeAudio(gd::String& audioName) {
for (auto resources : GetResources()) {
if (!resources) continue;

if (resources->HasResource(audioName) &&
resources->GetResource(audioName).GetKind() == "audio") {
// Nothing to do, the audio is a reference to a proper resource.
return;
}
if (resourcesManager->HasResource(audioName) &&
resourcesManager->GetResource(audioName).GetKind() == "audio") {
// Nothing to do, the audio is a reference to a proper resource.
return;
}

// For compatibility with older projects (where events were referring to files
Expand All @@ -80,14 +76,10 @@ void ArbitraryResourceWorker::ExposeAudio(gd::String& audioName) {
};

void ArbitraryResourceWorker::ExposeFont(gd::String& fontName) {
for (auto resources : GetResources()) {
if (!resources) continue;

if (resources->HasResource(fontName) &&
resources->GetResource(fontName).GetKind() == "font") {
// Nothing to do, the font is a reference to a proper resource.
return;
}
if (resourcesManager->HasResource(fontName) &&
resourcesManager->GetResource(fontName).GetKind() == "font") {
// Nothing to do, the font is a reference to a proper resource.
return;
}

// For compatibility with older projects (where events were referring to files
Expand All @@ -96,12 +88,7 @@ void ArbitraryResourceWorker::ExposeFont(gd::String& fontName) {
ExposeFile(fontName);
};

void ArbitraryResourceWorker::ExposeResources(
gd::ResourcesManager* resourcesManager) {
if (!resourcesManager) return;

resourcesManagers.push_back(resourcesManager);

void ArbitraryResourceWorker::ExposeResources() {
std::vector<gd::String> resources = resourcesManager->GetAllResourceNames();
for (std::size_t i = 0; i < resources.size(); i++) {
if (resourcesManager->GetResource(resources[i]).UseFile())
Expand All @@ -110,9 +97,6 @@ void ArbitraryResourceWorker::ExposeResources(
}

void ArbitraryResourceWorker::ExposeEmbeddeds(gd::String& resourceName) {
if (resourcesManagers.empty()) return;
gd::ResourcesManager* resourcesManager = resourcesManagers[0];

gd::Resource& resource = resourcesManager->GetResource(resourceName);

if (!resource.GetMetadata().empty()) {
Expand Down Expand Up @@ -176,6 +160,7 @@ void ArbitraryResourceWorker::ExposeResourceWithType(
}
if (resourceType == "tilemap") {
ExposeTilemap(resourceName);
ExposeEmbeddeds(resourceName);
return;
}
if (resourceType == "tileset") {
Expand All @@ -184,6 +169,7 @@ void ArbitraryResourceWorker::ExposeResourceWithType(
}
if (resourceType == "json") {
ExposeJson(resourceName);
ExposeEmbeddeds(resourceName);
return;
}
if (resourceType == "video") {
Expand Down Expand Up @@ -243,10 +229,12 @@ bool ResourceWorkerInEventsWorker::DoVisitInstruction(gd::Instruction& instructi
} else if (parameterMetadata.GetType() == "jsonResource") {
gd::String updatedParameterValue = parameterValue;
worker.ExposeJson(updatedParameterValue);
worker.ExposeEmbeddeds(updatedParameterValue);
instruction.SetParameter(parameterIndex, updatedParameterValue);
} else if (parameterMetadata.GetType() == "tilemapResource") {
gd::String updatedParameterValue = parameterValue;
worker.ExposeTilemap(updatedParameterValue);
worker.ExposeEmbeddeds(updatedParameterValue);
Copy link
Collaborator

Choose a reason for hiding this comment

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

why only those 2 types?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Only these kind of resources are depending on external files.

instruction.SetParameter(parameterIndex, updatedParameterValue);
} else if (parameterMetadata.GetType() == "tilesetResource") {
gd::String updatedParameterValue = parameterValue;
Expand Down
14 changes: 5 additions & 9 deletions Core/GDCore/IDE/Project/ArbitraryResourceWorker.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ namespace gd {
* \ingroup IDE
*/
class GD_CORE_API ArbitraryResourceWorker {
public:
ArbitraryResourceWorker(){};
public:
ArbitraryResourceWorker(gd::ResourcesManager &resourcesManager_)
: resourcesManager(&resourcesManager_){};
virtual ~ArbitraryResourceWorker();

/**
Expand All @@ -52,7 +53,7 @@ class GD_CORE_API ArbitraryResourceWorker {
* first to ensure that resources are known so that images, shaders & audio
* can make reference to them.
*/
void ExposeResources(gd::ResourcesManager *resourcesManager);
void ExposeResources();

/**
* \brief Expose a resource from a given type.
Expand Down Expand Up @@ -122,19 +123,14 @@ class GD_CORE_API ArbitraryResourceWorker {
*/
virtual void ExposeEmbeddeds(gd::String &resourceName);

protected:
const std::vector<gd::ResourcesManager *> &GetResources() {
return resourcesManagers;
};

private:
/**
* \brief Expose a resource: resources that have a file are
* exposed as file (see ExposeFile).
*/
void ExposeResource(gd::Resource &resource);

std::vector<gd::ResourcesManager *> resourcesManagers;
gd::ResourcesManager * resourcesManager;
};

/**
Expand Down
2 changes: 1 addition & 1 deletion Core/GDCore/IDE/Project/ObjectsUsingResourceCollector.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
namespace gd {

void ObjectsUsingResourceCollector::DoVisitObject(gd::Object& object) {
gd::ResourceNameMatcher resourceNameMatcher(resourceName);
gd::ResourceNameMatcher resourceNameMatcher(*resourcesManager, resourceName);

object.GetConfiguration().ExposeResources(resourceNameMatcher);
if (resourceNameMatcher.AnyResourceMatches()) {
Expand Down
22 changes: 12 additions & 10 deletions Core/GDCore/IDE/Project/ObjectsUsingResourceCollector.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
* reserved. This project is released under the MIT License.
*/

#ifndef ProjectObjectsUsingResourceCollector_H
#define ProjectObjectsUsingResourceCollector_H
#pragma once

#include <vector>

Expand All @@ -21,9 +20,10 @@ namespace gd {

class GD_CORE_API ObjectsUsingResourceCollector
: public ArbitraryObjectsWorker {
public:
ObjectsUsingResourceCollector(const gd::String& resourceName_)
: resourceName(resourceName_){};
public:
ObjectsUsingResourceCollector(gd::ResourcesManager &resourcesManager_,
const gd::String &resourceName_)
: resourcesManager(&resourcesManager_), resourceName(resourceName_){};
virtual ~ObjectsUsingResourceCollector();

std::vector<gd::String>& GetObjectNames() { return objectNames; }
Expand All @@ -33,12 +33,16 @@ class GD_CORE_API ObjectsUsingResourceCollector

std::vector<gd::String> objectNames;
gd::String resourceName;
gd::ResourcesManager *resourcesManager;
};

class GD_CORE_API ResourceNameMatcher : public ArbitraryResourceWorker {
public:
ResourceNameMatcher(const gd::String& resourceName_)
: resourceName(resourceName_), matchesResourceName(false){};
public:
ResourceNameMatcher(gd::ResourcesManager &resourcesManager,
const gd::String &resourceName_)
: resourceName(resourceName_),
matchesResourceName(false), gd::ArbitraryResourceWorker(
resourcesManager){};
virtual ~ResourceNameMatcher(){};

bool AnyResourceMatches() { return matchesResourceName; }
Expand Down Expand Up @@ -85,5 +89,3 @@ class GD_CORE_API ResourceNameMatcher : public ArbitraryResourceWorker {
};

}; // namespace gd

#endif // ProjectObjectsUsingResourceCollector_H
3 changes: 2 additions & 1 deletion Core/GDCore/IDE/Project/ProjectResourcesAdder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ namespace gd {
std::vector<gd::String> ProjectResourcesAdder::GetAllUseless(
gd::Project& project, const gd::String& resourceType) {
std::vector<gd::String> unusedResources;

// Search for resources used in the project
gd::ResourcesInUseHelper resourcesInUse;
gd::ResourcesInUseHelper resourcesInUse(project.GetResourcesManager());
gd::ResourceExposer::ExposeWholeProjectResources(project, resourcesInUse);
std::set<gd::String>& usedResources = resourcesInUse.GetAll(resourceType);

Expand Down
23 changes: 12 additions & 11 deletions Core/GDCore/IDE/Project/ProjectResourcesCopier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,28 +25,29 @@ bool ProjectResourcesCopier::CopyAllResourcesTo(
bool updateOriginalProject,
bool preserveAbsoluteFilenames,
bool preserveDirectoryStructure) {

gd::Project& project = originalProject;
if (!updateOriginalProject) {
std::shared_ptr<gd::Project> clonedProject(new gd::Project(originalProject));
project = *clonedProject;
}

Copy link
Owner

Choose a reason for hiding this comment

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

This seems incorrect and is probably linked to issue you may have encountered:

A reference can't be changed after it's set. So when you do gd::Project& project = originalProject, any further assignement will change the originalProject. I.e: when you do project = *cloneProject, you modify originalProject to be equal to clonedProject (which is itself made from originalProject).

When a reference is first assigned, it says to which object it points to. But then when you change it, it modifies the object itself, not the reference.

// Check if there are some resources with absolute filenames
gd::ResourcesAbsolutePathChecker absolutePathChecker(fs);
gd::ResourceExposer::ExposeWholeProjectResources(originalProject, absolutePathChecker);
gd::ResourcesAbsolutePathChecker absolutePathChecker(project.GetResourcesManager(), fs);
gd::ResourceExposer::ExposeWholeProjectResources(project, absolutePathChecker);

auto projectDirectory = fs.DirNameFrom(originalProject.GetProjectFile());
auto projectDirectory = fs.DirNameFrom(project.GetProjectFile());
std::cout << "Copying all resources from " << projectDirectory << " to "
<< destinationDirectory << "..." << std::endl;

// Get the resources to be copied
gd::ResourcesMergingHelper resourcesMergingHelper(fs);
gd::ResourcesMergingHelper resourcesMergingHelper(project.GetResourcesManager(), fs);
resourcesMergingHelper.SetBaseDirectory(projectDirectory);
resourcesMergingHelper.PreserveDirectoriesStructure(
preserveDirectoryStructure);
resourcesMergingHelper.PreserveAbsoluteFilenames(
preserveAbsoluteFilenames);

if (updateOriginalProject) {
gd::ResourceExposer::ExposeWholeProjectResources(originalProject, resourcesMergingHelper);
} else {
std::shared_ptr<gd::Project> project(new gd::Project(originalProject));
Copy link
Owner

@4ian 4ian Nov 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Note that I think this is a overly complicated way of writing:
gd::Project project = originalProject; // i.e.: you make a copy.

gd::ResourceExposer::ExposeWholeProjectResources(*project, resourcesMergingHelper);
}
gd::ResourceExposer::ExposeWholeProjectResources(project, resourcesMergingHelper);

// Copy resources
map<gd::String, gd::String>& resourcesNewFilename =
Expand Down
13 changes: 5 additions & 8 deletions Core/GDCore/IDE/Project/ResourcesAbsolutePathChecker.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
* Copyright 2008-2016 Florian Rival ([email protected]). All rights
* reserved. This project is released under the MIT License.
*/
#ifndef RESOURCESABSOLUTEPATHCHECKER_H
#define RESOURCESABSOLUTEPATHCHECKER_H
#pragma once

#include "GDCore/IDE/AbstractFileSystem.h"
#include "GDCore/IDE/Project/ArbitraryResourceWorker.h"
Expand All @@ -22,10 +21,10 @@ namespace gd {
*/
class GD_CORE_API ResourcesAbsolutePathChecker
: public ArbitraryResourceWorker {
public:
ResourcesAbsolutePathChecker(AbstractFileSystem& fileSystem)
: ArbitraryResourceWorker(),
hasAbsoluteFilenames(false),
public:
ResourcesAbsolutePathChecker(gd::ResourcesManager &resourcesManager,
AbstractFileSystem &fileSystem)
: ArbitraryResourceWorker(resourcesManager), hasAbsoluteFilenames(false),
fs(fileSystem){};
virtual ~ResourcesAbsolutePathChecker(){};

Expand All @@ -47,5 +46,3 @@ class GD_CORE_API ResourcesAbsolutePathChecker
};

} // namespace gd

#endif // RESOURCESABSOLUTEPATHCHECKER_H
5 changes: 3 additions & 2 deletions Core/GDCore/IDE/Project/ResourcesInUseHelper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@ std::set<gd::String> & usedImages = resourcesInUse.GetAllImages();
* \ingroup IDE
*/
class ResourcesInUseHelper : public gd::ArbitraryResourceWorker {
public:
ResourcesInUseHelper() : gd::ArbitraryResourceWorker(){};
public:
ResourcesInUseHelper(gd::ResourcesManager &resourcesManager)
: gd::ArbitraryResourceWorker(resourcesManager){};
virtual ~ResourcesInUseHelper(){};

std::set<gd::String>& GetAllImages() { return GetAll("image"); };
Expand Down
10 changes: 5 additions & 5 deletions Core/GDCore/IDE/Project/ResourcesMergingHelper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ namespace gd {
* \ingroup IDE
*/
class GD_CORE_API ResourcesMergingHelper : public ArbitraryResourceWorker {
public:
ResourcesMergingHelper(gd::AbstractFileSystem& fileSystem)
: ArbitraryResourceWorker(),
preserveDirectoriesStructure(false),
preserveAbsoluteFilenames(false),
public:
ResourcesMergingHelper(gd::ResourcesManager &resourcesManager,
gd::AbstractFileSystem &fileSystem)
: ArbitraryResourceWorker(resourcesManager),
preserveDirectoriesStructure(false), preserveAbsoluteFilenames(false),
fs(fileSystem){};
virtual ~ResourcesMergingHelper(){};

Expand Down
17 changes: 10 additions & 7 deletions Core/GDCore/IDE/Project/ResourcesRenamer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,16 @@ namespace gd {
*/
class ResourcesRenamer : public gd::ArbitraryResourceWorker {
public:
/**
* @brief Constructor taking the map from old name to new name.
* @param oldToNewNames_ A map associating to a resource name the new name to
* use.
*/
ResourcesRenamer(const std::map<gd::String, gd::String>& oldToNewNames_)
: gd::ArbitraryResourceWorker(), oldToNewNames(oldToNewNames_){};
/**
* @brief Constructor taking the map from old name to new name.
* @param oldToNewNames_ A map associating to a resource name the new name to
* use.
*/
ResourcesRenamer(gd::ResourcesManager &resourcesManager,
const std::map<gd::String, gd::String> &oldToNewNames_)
: gd::ArbitraryResourceWorker(resourcesManager),
oldToNewNames(oldToNewNames_){};

virtual ~ResourcesRenamer(){};

virtual void ExposeFile(gd::String& resourceFileName) override{
Expand Down
6 changes: 4 additions & 2 deletions Core/GDCore/IDE/Project/SceneResourcesFinder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,16 @@
namespace gd {

std::set<gd::String> SceneResourcesFinder::FindProjectResources(gd::Project &project) {
gd::SceneResourcesFinder resourceWorker;
gd::SceneResourcesFinder resourceWorker(project.GetResourcesManager());

gd::ResourceExposer::ExposeProjectResources(project, resourceWorker);
return resourceWorker.resourceNames;
}

std::set<gd::String> SceneResourcesFinder::FindSceneResources(gd::Project &project,
gd::Layout &layout) {
gd::SceneResourcesFinder resourceWorker;
gd::SceneResourcesFinder resourceWorker(project.GetResourcesManager());

gd::ResourceExposer::ExposeLayoutResources(project, layout, resourceWorker);
return resourceWorker.resourceNames;
}
Expand Down
3 changes: 2 additions & 1 deletion Core/GDCore/IDE/Project/SceneResourcesFinder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ class SceneResourcesFinder : private gd::ArbitraryResourceWorker {
virtual ~SceneResourcesFinder(){};

private:
SceneResourcesFinder() : gd::ArbitraryResourceWorker(){};
SceneResourcesFinder(gd::ResourcesManager &resourcesManager)
: gd::ArbitraryResourceWorker(resourcesManager){};

void AddUsedResource(gd::String &resourceName);

Expand Down
4 changes: 1 addition & 3 deletions Core/GDCore/IDE/ResourceExposer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,8 @@ void ResourceExposer::ExposeWholeProjectResources(gd::Project& project, gd::Arbi
// traverse the whole project (this time for events) and ExposeProjectEffects
// (this time for effects).

gd::ResourcesManager* resourcesManager = &(project.GetResourcesManager());

// Expose any project resources as files.
worker.ExposeResources(resourcesManager);
worker.ExposeResources();

project.GetPlatformSpecificAssets().ExposeResources(worker);

Expand Down
Loading