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

Chart editor no minimum for event values fix #4110

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

Lasercar
Copy link
Contributor

@Lasercar Lasercar commented Feb 6, 2025

Does this PR close any issues? If so, link them below.

Fixes #2861

Briefly describe the issue(s) fixed.

The default events didn't have minimum values set, lol.

Include any relevant screenshots or videos.

2025-02-06.18-11-41.1.mp4

Sigh, if I had noticed that I didn't change it to the develop branch I wouldn't of deleted the branch and perma closed the previous PR out of a worry that I did something wrong on my end.

lol, lmao, imagine coding a input to have minimum values but just.. not setting a value?
@github-actions github-actions bot added status: pending triage Awaiting review. pr: haxe PR modifies game code. size: small A small pull request with 10 or fewer changes. and removed status: pending triage Awaiting review. labels Feb 6, 2025
@Lasercar
Copy link
Contributor Author

Lasercar commented Feb 6, 2025

Oh no, the bot's out of it again

@Hundrec Hundrec added the status: pending triage Awaiting review. label Feb 6, 2025
@Hundrec
Copy link
Collaborator

Hundrec commented Feb 6, 2025

I suggest that you remove the comment where you signed your name. We don't want signatures in the source code!

@Hundrec Hundrec added type: minor bug Involves a minor bug or issue. status: needs revision Cannot be approved because it is awaiting some work by the contributor. and removed status: pending triage Awaiting review. labels Feb 6, 2025
@Lasercar
Copy link
Contributor Author

Lasercar commented Feb 6, 2025

Oh, sorry.

I saw a signature elsewhere in the code and thought it'd be alright.

@Hundrec
Copy link
Collaborator

Hundrec commented Feb 6, 2025

Thank you, though I do think the whole comment should be removed. It doesn't provide informational value to people reading the code.

Also, please let us know where the other signature is so we can take a look.

@Lasercar
Copy link
Contributor Author

Lasercar commented Feb 6, 2025

Also, please let us know where the other signature is so we can take a look.

It's in the characterselect substate.

I could remove it and my ones in my update of that state? #4072

@Hundrec
Copy link
Collaborator

Hundrec commented Feb 6, 2025

Please don't include unrelated changes in this PR.

Can you link the line to me? I'm digging around for it but will probably take a while to find it

@Lasercar
Copy link
Contributor Author

Lasercar commented Feb 6, 2025

@Hundrec
Copy link
Collaborator

Hundrec commented Feb 6, 2025

Thanks! Cheems is closer to the Funkin' Crew than the general community is, so that signature is acceptable.
In general, community PR comments should be informative and unsigned.

@Lasercar
Copy link
Contributor Author

Lasercar commented Feb 6, 2025

Thanks! Cheems is closer to the Funkin' Crew than the general community is, so that signature is acceptable. In general, community PR comments should be informative and unsigned.

Could that be added into the style guide? Thanks.

@Hundrec
Copy link
Collaborator

Hundrec commented Feb 6, 2025

Good idea!
When we create a PR guide, we'll be sure to include a point about signatures.

@Lasercar
Copy link
Contributor Author

Lasercar commented Feb 6, 2025

The style guide already exists: https://github.com/FunkinCrew/Funkin/blob/main/docs/style-guide.md

It just isn't exactly linked to or anything, you just have to find it in the docs folder.

@Hundrec
Copy link
Collaborator

Hundrec commented Feb 6, 2025

Oh! You meant that old thing...
Thanks for reminding me, we could actually link this for people to read in the coming days. Cool stuff!

Copy link
Collaborator

@AbnormalPoof AbnormalPoof left a comment

Choose a reason for hiding this comment

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

LGTM

@Lasercar Lasercar requested a review from Hundrec February 12, 2025 14:29
@Hundrec Hundrec added status: pending triage Awaiting review. and removed status: needs revision Cannot be approved because it is awaiting some work by the contributor. labels Feb 12, 2025
@Lasercar Lasercar changed the base branch from develop to main February 22, 2025 01:08
@Lasercar Lasercar changed the base branch from main to develop February 22, 2025 01:09
Copy link
Collaborator

@Hundrec Hundrec left a comment

Choose a reason for hiding this comment

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

Everything works except for the Rate value in the Set Camera Bop event.

Not sure why, but it's locked at 0!

image

@Lasercar Lasercar requested a review from Hundrec February 22, 2025 23:04
Copy link
Collaborator

@Hundrec Hundrec left a comment

Choose a reason for hiding this comment

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

Very good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: haxe PR modifies game code. size: small A small pull request with 10 or fewer changes. status: pending triage Awaiting review. type: minor bug Involves a minor bug or issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG REPORT: Negative Scroll Speed Glitch
3 participants