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

Support ADDON_DATA in addon_config.mk #190

Merged
merged 6 commits into from
May 30, 2019

Conversation

hiroMTB
Copy link
Contributor

@hiroMTB hiroMTB commented Sep 25, 2018

original PR

#92 by @mattfelsen in 2015-16.

additional commit

I checked original PR still works nicely but only able to copy single file.
I added folder copy feature so that we can avoid listing up all files.
Tested on OSX and Windows, both of commandLine and frontend app works okay.

Known problem

If we have same data folder/file in different addon, PG will copy both to /myProject/bin/data but overwrite. For example, considering we have following two addons.

  • ofxCoolShaders
    • /src
    • /data/img
  • ofxCoolPhotos
    • /src
    • /data/img

And generate a project which copies both of /data/img. Then /myProject/bin/data/img folder will be overwritten by last addon specified. This might be solved by adding new "merge" feature to ofDirectory class in order to avoid overwrite folder but merge contents.

@arturoc arturoc merged commit 96d97bb into openframeworks:master May 30, 2019
@arturoc
Copy link
Member

arturoc commented May 30, 2019

thanks!

@hiroMTB
Copy link
Contributor Author

hiroMTB commented May 30, 2019

Please notice this can cause problem when user "update" existing project.

It will overwrite his/her own data resource if the folder name is the same.

@arturoc
Copy link
Member

arturoc commented Jun 1, 2019

yeah i saw. i guess most people add addons at the beginning before having any data? also data from addons would usually be named in a way that it's obvious won't clash easily with other files i can imagine

in any case perhaps we could add a check to see if those files exist and fail it they do?

@hiroMTB
Copy link
Contributor Author

hiroMTB commented Jun 1, 2019

I often start a new project and update several times whenever I noticed I need an additional addon. (or I decide I don't need addon already imported)

also data from addons would usually be named in a way that it's obvious won't clash easily with other files I can imagine

I expect we can find /data/shader in several addons, also /data/png etc?

in any case perhaps we could add a check to see if those files exist and fail if they do?

We can check existing data folder and cancel copy. But then addon does not work properly because of missing files. I think the best solution is implementing the "merge" feature on ofDirectory. Or we ask addon makers to use a unique folder name. (this sounds a bad idea)

@hiroMTB
Copy link
Contributor Author

hiroMTB commented Jun 1, 2019

Or we can disable folder copy and only allow single file copy (like an original PR).
This case we should list up all files in addon.config with ADDON_DATA.

@arturoc
Copy link
Member

arturoc commented Jun 1, 2019

when i say fail i mean it should show an error so you know that the addon didn't install properly. and yes i think addons with data should have unique folder names like ofxSomething_shaders instead of shaders

@hiroMTB
Copy link
Contributor Author

hiroMTB commented Jun 1, 2019

I see, indeed just skipping is a bad idea.
So, we have four solutions so far.

  • A. Only copy folder if the same folder does not exist ( and return an error, fail)
  • B. Always copy folder which has a unique name. (ofxSomething_shaders, overwrite)
  • C. Always copy a single file (overwrite)
  • D. Always copy with merge feature (overwrite)

I start thinking B is the best.
It makes easy to notice which data come from which addon.
If there is old addon with /data/shader included, probably it does not have ADDON_DATA feature in addon.config. So it won't overwrite user data.

+edit
And we can put a filter in commandLine PG which only copy addonName + "_" + dataFolderName

@hiroMTB hiroMTB deleted the feature-copy-ADDON_DATA branch September 16, 2019 21:40
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.

3 participants