clean merge ...

[jgarza9788]

notes: i might need to re-do the gif
it's good, but maybe issues with the Nintendo copy-right stuff.

note: meld is a great program to compare files and folders.

[jgarza9788]

ok, seems like i need to add more comments.
because it's failing

[jgarza9788]

a few more comment precents...
Percentage of Comments in Base Repository: 30.6
Percentage of Comments after Merge: 30.2

[jgarza9788]

note to @Schneegans
the clang-format tool/script might need to be updated ... since i had to make manual changes to get it to pass.

[jgarza9788]

Yay it worked.
let me know if you want a clean clean branch 😅

[Schneegans]

Awesome, once I find the time, I'll have a closer look at this! I think I'll squash-merge this so no need to clean up the history. Thank you already so much!

[jgarza9788]

Yeah, you didn’t push me to get it done. So no need to push you to merge. I hope you like what I did. 👍 *Justin Garza* (818) 251-0647 / ***@***.***

…

On Mon, Dec 2, 2024 at 7:46 AM Simon Schneegans ***@***.***> wrote: Awesome, once I find the time, I'll have a closer look at this! I think I'll squash-merge this so no need to clean up the history. Thank you already so much! — Reply to this email directly, view it on GitHub <#479 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB5URPT75FGVW7WARO5DL632DRJCFAVCNFSM6AAAAABSZ6AYNOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMJRGQ2DQNBWG4> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[Schneegans]

Hi there, thanks again! I haven't looked into the details yet, but I already wanted to provide some high-level feedback.

Besides the few comments I made to the files, I have three observations:

• "4Point Stars (Sparks)" is a very weird name. I would just call them "Sparks". In the code and in the UI:
  • Show sparks
  • Spark Count
  • Spark Color
• The other stars are called sometimes "stars", sometimes "5pstars", and sometimes "star rings". Maybe they could simply be called stars? So in the UI we would have
  • Show Stars
  • Star Ring Count
  • Star Ring Rotation
  • Star Count Per Ring
• Overall, I have the feeling that some of the options are a bit "too much". There's really no need to adjust the spark number to 100. All the star options have good defaults and I think anything else (more stars, faster or slower rotation) makes it look significantly worse. Why should anybody set the rotation to 5? While you could leave it like it is currently (would be fine for me), I think that the effect could actually benefit from less options.

Anyways, thanks a lot for your work!

Later I will have a ore detailed look at the code!

[Schneegans]

Could you remove this? I think it's very specific to you 😄

Suggested change

	.VSCodeCounter/2024-12-01_14-24-39/results.txt
	.VSCodeCounter/2024-12-01_14-24-39/results.md
	.VSCodeCounter/2024-12-01_14-24-39/results.json
	.VSCodeCounter/2024-12-01_14-24-39/results.csv
	.VSCodeCounter/2024-12-01_14-24-39/diff.txt
	.VSCodeCounter/2024-12-01_14-24-39/diff.md
	.VSCodeCounter/2024-12-01_14-24-39/diff.csv
	.VSCodeCounter/2024-12-01_14-24-39/diff-details.md
	.VSCodeCounter/2024-12-01_14-24-39/details.md
	.VSCodeCounter/2024-12-01_14-22-46/results.txt
	.VSCodeCounter/2024-12-01_14-22-46/results.md
	.VSCodeCounter/2024-12-01_14-22-46/results.json
	.VSCodeCounter/2024-12-01_14-22-46/results.csv
	.VSCodeCounter/2024-12-01_14-22-46/diff.txt
	.VSCodeCounter/2024-12-01_14-22-46/diff.md
	.VSCodeCounter/2024-12-01_14-22-46/diff.csv
	.VSCodeCounter/2024-12-01_14-22-46/diff-details.md
	.VSCodeCounter/2024-12-01_14-22-46/details.md

[Schneegans]

What did you change exactly in this file? If it's just formatting, it would be cool if you could undo it 😄

[Schneegans]

Thanks for attempting to comply with the comment-percentage check. But I think we do not have to enforce this here. I guess it did do it's job - you tried to add many comments to the code, you even added some to the focus effect! But I think we do not need to overdo it 😄

So I think you could remove this here (you have enough explanation above) and then we can ignore the failing check.

[jgarza9788]

Thanks for the comment,
And appreciating the work.

1. Simplify parameters
   Make the names 2-3 words or less.
   Since they can see the effect there is no reason to be exact.

2. merge parameters if possible
   Show/enable is not needed if the count can be set to 0
   ^ this can decrease the number of parameters

3. Limit the max number some parameters can be if it seems too much (ie 100 sparks or 5x rotations)

I think these are good takeaways for me.

[jgarza9788]

Should I split up the effects?
Mushroom 8bit and Mushroom ?

[Schneegans]

Good question. Alternatively, you could keep it here but do not disable all other options. So people could combine the 8bit-scale animation with the stars. Would this be possible? So simply call the checkbox "8bit Animation" maybe with a subtitle "Use an old-school animation for the window's size" and leave all other options available even if this one is checked.

[jgarza9788]

You are the owner... so I can implement your decision.

🫡

[jgarza9788]

I'll close the commit until, i am able to provide what you want.
thanks for the feedback and let me know if you want to split mushroom or (do what you suggested)
and i'll get some other effects ready in the meantime :) (trying to stay busy)

[Schneegans]

I think you can keep this as one effect. No need to split it 😄