-
Notifications
You must be signed in to change notification settings - Fork 7.1k
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
Some fixes for search paths: #17435
Some fixes for search paths: #17435
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -442,6 +442,11 @@ class CC_DLL FileUtils | |
*/ | ||
virtual void setSearchPaths(const std::vector<std::string>& searchPaths); | ||
|
||
/** | ||
* Get default resource root path. | ||
*/ | ||
const std::string& getDefaultResourceRootPath() const; | ||
|
||
/** | ||
* Set default resource root path. | ||
*/ | ||
|
@@ -457,12 +462,21 @@ class CC_DLL FileUtils | |
/** | ||
* Gets the array of search paths. | ||
* | ||
* @return The array of search paths. | ||
* @return The array of search paths which may contain the prefix of default resource root path. | ||
* @note In best practise, getter function should return the value of setter function passes in. | ||
* But since we should not break the compatibility, we keep using the old logic. | ||
* Therefore, If you want to get the original search paths, please call 'getOriginalSearchPaths()' instead. | ||
* @see fullPathForFilename(const char*). | ||
* @lua NA | ||
*/ | ||
virtual const std::vector<std::string>& getSearchPaths() const; | ||
|
||
/** | ||
* Gets the original search path array set by 'setSearchPaths' or 'addSearchPath'. | ||
* @return The array of the original search paths | ||
*/ | ||
virtual const std::vector<std::string>& getOriginalSearchPaths() const; | ||
|
||
/** | ||
* Gets the writable path. | ||
* @return The path that can be write/read a file in | ||
|
@@ -873,6 +887,11 @@ class CC_DLL FileUtils | |
*/ | ||
std::vector<std::string> _searchPathArray; | ||
|
||
/** | ||
* The search paths which was set by 'setSearchPaths' / 'addSearchPath'. | ||
*/ | ||
std::vector<std::string> _originalSearchPaths; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why need _originalSearchPaths There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FileUtils uses If If current search path is auto oldDefaultResourceRoot = fileUtils->getDefaultResourceRootPath(); // Suppose it return "" which is an empty string.
auto oldSearchPaths = fileUtils->getSearchPaths(); // oldSearchPaths is : ["bbb/", "ccc/"]
fileUtils->setDefaultResouceRootPath("aaa"); // oldSearchPath is : ["aaa/bbb/", "aaa/ccc"]
// oldSearchPaths will be modified by adding a prefix `aaa` since `_searchPathArray` was modified.
... ...
// reset resource root
fileUtils->setDefaultResourceRootPath(oldDefaultResourceRoot);
auto newSearchPath = fileUtils->getSearchPaths(); // newSearchPath is incorrect: ["aaa/bbb", "aaa/ccc"] since it returns `_searchPathArray` which was added by prefix 'aaa' |
||
|
||
/** | ||
* The default root path of resources. | ||
* If the default root path of resources needs to be changed, do it in the `init` method of FileUtils's subclass. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sbrednikhin, @balmerdx
So the key point is that this line push the
default resource path
to the end, right?Yes, I know I broke the compatibility here since I think the search path of default resource should always be at the end. I don't know what kinds of situation you will do in your game. Perhaps, more code snippets and more detail are welcome.
Anyway, if you think I was wrong, feel free to submit a
new
orbetter
pull request to correct my mistake. Thanks.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't care where default resource path is, actually. My only point is that if you breaking regular code behavior, causing people to fix their code (which must rely on some order of paths), you should have some good reason for that. Not just thinking that it is better.
Since at least some people don't see any obvious reason for that change, you could point on it in comment or something.
This project is open source common code - not your own branch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK,you're totally right, thanks for reminding me. I will revert that line. I was wrong, I used it as 'my own my private branch', good luck guys, thank you guys.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, for understanding. We are very appreciate your work.