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

rlottie: now loadFromData() api takes resourcePath for loading external resource #3

Merged
merged 1 commit into from
Mar 5, 2019

Conversation

smohantty
Copy link
Contributor

#2

@smohantty smohantty requested a review from hermet March 4, 2019 04:37
Copy link
Member

@hermet hermet left a comment

Choose a reason for hiding this comment

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

In API point of view, how about an static api to set global resource path?
It could be easier to use rather then setting resource path every time.
ex)
static rlottie::Config::defaultResPath("../res/lottie");
static rlottie::Animation::defaultResPath(...);
whatever default/global/(skip), Res/Resource, Path/Dir ...

And here resource path could be shared through lottie font, image, etc.

What do you think?

@smohantty
Copy link
Contributor Author

i thought about the static api, the problem is this is a library and can be used by multiple client inside the same application . then if they have different resource path they will conflict with each other.(one will override other)

And because the resourcePath argument is default , most of time you don't have to specify resource path , if you don't have external resource.

so i think passing the argument in the api will give more flexibility .

@hermet
Copy link
Member

hermet commented Mar 5, 2019

As the ui app, multiple clients inside a same app? if multiple clients as multiple process, it doesn't any problems. If you mean multiple clients in a process, they must be packaged in one space. It means the process definitely needs share the resource and data (that's why they are using it). That case, it doesn't really need to have separate environments. We should consider convenient apis for the major scenario.

@smohantty
Copy link
Contributor Author

For example, application has 2 lottie player instance one using the theme resource (for example through edc) and other one just set a file path from the system (it could haven from 2 different thread as well) . In that case how this will guarantee the last resource set is still valid .. someone else might change that .
Anyway static api that changes changes global data is always a bad idea (in a multi thread environment).
So i don't think there is any convenient about adding a static api . it will just add more subtle bugs .
Passing explicitly when needed is much cleaner approach in my opinion.

@hermet
Copy link
Member

hermet commented Mar 5, 2019

then go on please.

@hermet hermet merged commit b7d4886 into Samsung:master Mar 5, 2019
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.

2 participants