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

[FEATURE] Scroll Speed Event #2409

Merged
merged 9 commits into from
May 30, 2024

Conversation

Burgerballs
Copy link
Contributor

@Burgerballs Burgerballs commented May 9, 2024

2024-05-09.17-53-43.online-video-cutter.com.mp4

What this PR does is implement a new event that sets the current scroll speed. As well as a public scrollSpeed variable in Strumline. Without adding any sustain-related rendering issues.

image

This PR also adds a parentStrumline variable to SustainTrail, and a way for it to redraw once the scroll speed changes.

2024-05-10.21-21-08.online-video-cutter.com.mp4

The event can also affect either strumline, an example of this is the video above!

image
This event can also set the base scroll directly instead of acting as a multiplier (disabled by default)!

Setting the value to 1 with Absolute disabled will set the speed to the default. (important)

PS; i ACTUALLY tested it this time

@Burgerballs Burgerballs marked this pull request as draft May 10, 2024 17:08
@Burgerballs
Copy link
Contributor Author

Converted to draft in order to add an extra cool feature. Changing the speed of a single strumline!

@amyspark-ng
Copy link

shouldn't it be just to set the new scroll speed? i think that'd be nicer...

@Burgerballs
Copy link
Contributor Author

shouldn't it be just to set the new scroll speed? i think that'd be nicer...

Noted, making changes right now

No longer Additive, it just sets the scroll speed

Now you can set it to either strumline
@Burgerballs Burgerballs changed the title Additive Scroll Speed with event and variable Scroll Speed with event and variable May 10, 2024
@Burgerballs Burgerballs marked this pull request as ready for review May 10, 2024 20:28
@Burgerballs
Copy link
Contributor Author

I have now undrafted it lol

@Burgerballs Burgerballs changed the title Scroll Speed with event and variable Scroll Speed Event May 10, 2024
@amyspark-ng
Copy link

This is pretty awesome

@Burgerballs
Copy link
Contributor Author

is there anything else that should be added to this or is it all good

@Burgerballs Burgerballs changed the title Scroll Speed Event [FEATURE] Scroll Speed Event May 14, 2024
@ninjamuffin99
Copy link
Member

this is pretty awesome ! Gonna ping @EliteMasterEric for his thoughts on how it's implemented code wise, but def something we can use in places ! Thanks!

@Burgerballs
Copy link
Contributor Author

this is pretty awesome ! Gonna ping @EliteMasterEric for his thoughts on how it's implemented code wise, but def something we can use in places ! Thanks!

Thank you!

@EliteMasterEric
Copy link
Member

Once I can check the code more thoroughly I can give more feedback, but I like this in general.

Have you tried seeing how it reacts visually when changing scroll speed while notes are visible? The tween should keep it from glitching but I'm curious how it looks.

Also, 1.0 should reset to the base scroll speed, not 0.0.

Maybe there could also be an absolute checkbox (unchecked by default) that sets the value rather than applying a multiplier?

@Burgerballs
Copy link
Contributor Author

Burgerballs commented May 15, 2024

Once I can check the code more thoroughly I can give more feedback, but I like this in general.

Have you tried seeing how it reacts visually when changing scroll speed while notes are visible? The tween should keep it from glitching but I'm curious how it looks.

Also, 1.0 should reset to the base scroll speed, not 0.0.

Maybe there could also be an absolute checkbox (unchecked by default) that sets the value rather than applying a multiplier?

Sounds like good ideas, i'll try to put them in, unrelated but in it's current state the event sets the absolute speed, I will add multiplication tho as it seems easy to do on paper.

@Burgerballs
Copy link
Contributor Author

don e

@Burgerballs
Copy link
Contributor Author

the sustains were broke but now they not :D

@Burgerballs
Copy link
Contributor Author

@EliteMasterEric once ur able to check the code could you make sure to tell me if theres anything wrong

@EliteMasterEric
Copy link
Member

I've been busy with other tasks lately, I will definitely look into reviewing it soon-ish!

If it looks good and doesn't have any problematic bugs, I will try to merge it to have it available for the next content update.

@Burgerballs
Copy link
Contributor Author

I've been busy with other tasks lately, I will definitely look into reviewing it soon-ish!

If it looks good and doesn't have any problematic bugs, I will try to merge it to have it available for the next content update.

Alr!

@Burgerballs
Copy link
Contributor Author

:p

@EliteMasterEric EliteMasterEric added the enhancement New feature or request label May 30, 2024
@EliteMasterEric
Copy link
Member

Finally got around to testing this and it worked great!

@EliteMasterEric EliteMasterEric merged commit 6cc9c3d into FunkinCrew:develop May 30, 2024
@Burgerballs Burgerballs deleted the scroll-speed-fix branch May 30, 2024 11:21
@EliteMasterEric EliteMasterEric added this to the 0.4.0 milestone Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants