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

Improved randomizer logic #26

Merged
merged 4 commits into from
Jul 9, 2022
Merged

Improved randomizer logic #26

merged 4 commits into from
Jul 9, 2022

Conversation

Loeffeldude
Copy link
Contributor

@Loeffeldude Loeffeldude commented Jul 9, 2022

This should resolve #11. I am not sure what default time it should be so I settled on 1000. I also added a pauseDuration argument that determines how long it should hold on one option.

If you are not happy with the defaults I can change them.

You could make the animation look nicer later by increasing the pause duration the closer to the end we are to give it a kind of slot machine effect. Let me know what you think about that.

@dcallus
Copy link
Contributor

dcallus commented Jul 9, 2022

This is great. I love it. The code is really nice. It seems to run smoother than mine too. My code was a little janky but it worked, but this is much nicer to read and seems more correct. Thank you for the comments in the code. It's helped me understand what's going on and will help others.

I think the speed and duration are perfect by the way and I definitely like the ability to change the overall duration as well as the duration of each item. This is more than I could have hoped for.

By the way that slowing down slot machine idea sounds super cool. Would you like to give that a try and add it to this PR? Or should I just merge this and maybe you can try that in a separate branch, it's up to you?

Also, you have a typo, the word DEFAULT in line 4 and 55. Obviously I can easily fix this, no bother, but in case you're going to add to this file anyway I thought I'd let you know.

:) Really appreciate the help. Just with that slot-machine style idea though, I think you 'get it'.

Good work 👍 👍 👍

@dcallus dcallus self-requested a review July 9, 2022 17:25
@Loeffeldude
Copy link
Contributor Author

I can add the slot machine mechanic right now and fix the typo too. Thanks for all the kind words.

@dcallus
Copy link
Contributor

dcallus commented Jul 9, 2022

I can add the slot machine mechanic right now and fix the typo too. Thanks for all the kind words.

legend!

@Loeffeldude
Copy link
Contributor Author

Ok the slowdown was a lot more complicated then I thought it would be but I think I got it feeling right. Let me know what you think. No hard feelings if you don't end up using it in the end.

Copy link
Contributor

@dcallus dcallus left a comment

Choose a reason for hiding this comment

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

Absolutely epic! Love it - it definitely looks way cooler! Thanks so much for this. Really appreciate it and the time you took to make it :)

@dcallus dcallus merged commit 49e5e3b into fillmaster:develop Jul 9, 2022
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.

Improve Randomiser logic
2 participants