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

[Needs Review] implementing backward compatible follow multi-target #30

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

dev-masih
Copy link
Contributor

@dev-masih dev-masih commented Nov 7, 2019

So what I did was move target tables to camera module instead of camera script but kept target properties inside camera script because removing it cases a big compatibility issue (forcing devs to select follow target only with code). but the value of this property ignores and inside camera script init function it's value will be sent to targets that's inside camera module.
the only issue is if someone changes the target property directly in the middle of the game with go property instead of using follow function call or msg, the changed value will not affect anything

and it needs a better example implementation

@dev-masih dev-masih marked this pull request as ready for review November 7, 2019 09:01
@britzl
Copy link
Owner

britzl commented Nov 11, 2019

I like the idea and the implementation makes sense I think. Do you have any additional thoughts or any concerns with the proposed solution?

@dev-masih
Copy link
Contributor Author

I like the idea and the implementation makes sense I think. Do you have any additional thoughts or any concerns with the proposed solution?

not until table support for go property gets added to the engine.
and the multi-target i think needs a better example inside allfeature (maybe an extra button) and parts of doc needs fixing

@dev-masih
Copy link
Contributor Author

@britzl if you think this implementation is fine, tell me so that i begin to fix example and docs

@britzl
Copy link
Owner

britzl commented Nov 17, 2019

@dev-masih Yes, I like it. Please go ahead!

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