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

Allow Tweens with duration 0 and above #29568

Closed
wants to merge 0 commits into from

Conversation

DevinPentecost
Copy link
Contributor

Hi,

This is the change that appears to work to resolve the following issue:
#29567

I've built it and confirmed that it works. However I do not know how to run unit tests to confirm it does not break anything else. Additionally, I'd like to add a unit test for this particular condition if possible to make sure it is not broken in the future.

If you could please direct me to how to ensure this fix does not break tests and how to add my own test to this pull request I'd appreciate it.

Thanks,
-Devin

@Calinou
Copy link
Member

Calinou commented Jun 7, 2019

However I do not know how to run unit tests to confirm it does not break anything else. Additionally, I'd like to add a unit test for this particular condition if possible to make sure it is not broken in the future.

The short answer is that we don't add unit tests for new features; see discussion in #2641 🙂

There are a few built-in tests in place, but they don't function like traditional unit tests for the most part (they're mostly here to test engine features when porting Godot to a new platform).

@bojidar-bg
Copy link
Contributor

There are 5 other instances of p_duration <= 0 in the same file. Shouldn't they be changed as well?

@DevinPentecost
Copy link
Contributor Author

DevinPentecost commented Jun 7, 2019 via email

@DevinPentecost
Copy link
Contributor Author

I added a bunch of comments and did some light refactoring...
Hopefully I didn't break too much with it, I'll try to take a look later tonight to verify more

@DevinPentecost
Copy link
Contributor Author

DevinPentecost commented Jun 7, 2019

Fixed merge issue, waiting on builds...

@DevinPentecost
Copy link
Contributor Author

Making a few more fixes, I'll keep working on them over the weekend

@DevinPentecost
Copy link
Contributor Author

Hi,

I think I've done all I intend to do with this PR. Please review the changes.

Thanks!
-Devin

@Chaosus
Copy link
Member

Chaosus commented Jun 8, 2019

You need to squash commits together as described in http://docs.godotengine.org/en/latest/community/contributing/pr_workflow.html#mastering-the-pr-workflow-the-rebase Be careful if you didnt do it before (better test it on one of your local repositories)

@DevinPentecost
Copy link
Contributor Author

Hi,

I'm working on squashing the commits. I may need to reopen the PR with a new branch.

Thanks,
-Devin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants