Try to clean up asset folders for sample apps. #128
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This removes the build step where we copy a subset of assets, and makes it so that FilamentApp hands out a "root path" for assets. For now this is determined based on the location of the executable. This allows developers to launch samples from any CWD. Closes #11
romainguy
reviewed
Aug 23, 2018
samples/app/FilamentApp.h
Outdated
| // Returns the path to the Filament root for loading assets. For now this is determined by | ||
| // looking up from the executable folder, which allows users to launch samples from any folder. | ||
| static const utils::Path& getRootPath() { | ||
| static const utils::Path root = utils::Path::getCurrentExecutable().getAncestor(3); |
There was a problem hiding this comment.
This assumes we always run samples from out/xxx/samples. This will break if someone builds manually with cmake or if we distribute the samples in a zip file.
There was a problem hiding this comment.
Yup, and we were already making this assumption in some places, this merely consolidates the badness.
Maybe I should add back the asset-copying stuff to CMake, which would allow me to replace getAncestor with getParent...
There was a problem hiding this comment.
I think that's the best solution.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This removes the build step where we copy a subset of assets, and makes
it so that FilamentApp hands out a "root path" for assets. For now this
is determined based on the location of the executable. This allows
developers to launch samples from any CWD.
Closes #11