Ideas for PRs

[Daniel-Cortez]

Readme.md says that PRs should be discussed first before submitting them. I apologize that I didn't do so before submitting my previous PRs, as usually projects that have this kind of rule only apply it for PRs that add new features, not bugfixes, which is why at first I assumed that's the case here too.

Anyway, here's the list of changes I'd like to discuss before submitting them as PRs.

• Make a global list of character names.
  In HUD.gd, move the characterNames array into Global.gd (next to the Global.CHARACTERS enum) and make the other scripts (HUD.gd, GoalPost.gd etc.) reuse it.
  Also, fill the array automatically based on the names of members of Global.CHARACTERS. This is possible due to enum in GDScript being just a syntax sugar for dictionaries, which is why we can obtain all names with CHARACTERS.keys(), exclude CHARACTERS.NONE and make only the first letter capital using String.capitalize(). This way the user won't even need to touch that array when modifying the character list in Global.CHARACTERS.

• Implement "Robotnik" monitor.
  Pretty self-explanatory, I guess.
  Another thing is that these "magic numbers"
  
  
  could be replaced with named constants, so the users won't have to manually replace the IDs in code when adding new kinds of items.

• Fix time interval for generation of air bubbles.
  

  SonicWorldsNext/Scripts/Gimmicks/BubbleGenerator.gd

  Line 33 in 0e2bd46

  bigBubbleTimer = 10

  
  Clearly the intention here was to generate bubbles each 10 seconds, but since timings between _process() calls are not precise, the resulting interval becomes slightly more than 10 s (it varies between 10.0 and 10.0166666... seconds).
  This can be fixed by simply replacing = with += (so it would be bigBubbleTimer += 10.0) - this way we'll subtract the "overflow" part of the current interval from the interval for the next bubble, so the average time will always be around 10.0 seconds. For example, if the current interval turns out to be 10.015 s, next time we'll wait for 9.985 s.
  Also, maybe we could replace "magic numbers" for bubble types with named constants?
  
  

• Reorganize data in Animal.gd
  Currently the data for animals is scattered all over the place: the physics data is in animalPhysics at the beginning of the script, the frame data is hardcoded in _ready() and the gravity type (flap or bounce) is hardcoded in _physics_process().
  This can be reorganized in a much more convenient way:
  
  
  
  Actually there's a lot more places where magic numbers could be replaced with named constants for better readability and maintainability. Among what I could also notice are Spring.gd (spring types and directions), LayerSwitcher.gd (orientation types), Global.gd/Player.gd/Monitor.gd/SuperStart.gd (IDs for song themes: invincible (0), speed up (1), stage clear (2)), etc.

• Replace all occurrences of 60 frames with Engine.get_physics_frames().
  For example, function div_by_delta() from GlobalFunctions.gd could be modified like this:
  
  This will prepare the codebase for future increase of physics update rate (see Score countdown is framerate dependent #236 (comment)).

Are those ideas good or not? Can something of the above be done in a better way? I'd be happy to hear any constructive criticism and/or opinions on this.

[Daniel-Cortez]

Also, this might be a bit off-topic, but still, what is this "boost" item? Couldn't find anything about it on the web; googling that icon also doesn't give anything reasonable.

[Techokami]

It's for people that want to add the boost mechanics; this is the item icon from Sonic Rush Adventure for the item box that fills the boost gauge.

[DimensionWarped]

For what it's worth, I disagree with needing to have a 'Discussion' as a requirement for a PR. A PR is a discussion in my eyes. That said, it can definitely be worth having a discussion before you set out to do a certain set of work because it might be something someone else is already working on or will be significantly be impacted by that work. As long as you don't mind submitting a PR only to be told "It's going to step on too many toes, hold off and rebase later", feel free to go straight to PRs.

My in progress Player Refactoring patch is going to preclude any work on things like character names. Those are going to be moved into the PlayerAvatar (name still not totally set in stone, but PlayerChar was taken by Player.gd) class which is basically the individual character sprite controller scenes given a common class. A lot of stuff about characters is changing right now, so I'd honestly just avoid working on anything too core to that until I'm finished. Feel free to post recommendations in this thread though and if it's something I like I'll go ahead and incorporate it.

EDIT: It's looking like the name of the character probably isn't really going to be one of those elements that gets moved to PlayerAvatar stuff. I went ahead and added a global method of getting a character's name from the enum though.

I absolutely agree on the use of magic numbers, but that's going to require a lot of work to decide when they are and aren't OK. My impression is that rules like this should only be policed for truly global things while for stuff like gimmicks and the like which are more isolated it's not that big of a deal.

As for the Robotnik monitor, it's almost a trivial add other than our monitors currently not explicitly giving each item their own frame and lives in particular being different from the others thanks to the myriad of portraits. I'd request that you hold off on that one for the time being only because monitor is slightly impacted by the player refactoring patch.

On bubble generator, you have my enthusiastic support to take that one on.

Same for animals

[Daniel-Cortez]

On bubble generator, you have my enthusiastic support to take that one on.

Same for animals

Done, PRs are awaiting review.

A lot of stuff about characters is changing right now, so I'd honestly just avoid working on anything too core to that until I'm finished.

Yeah, judging from what you're saying, trying to modify characterNames or change any other core stuff wouldn't be very productive for now.

In any case, I have a few questions regarding your in-progress refactoring (as well as some potential suggestions), but they kind of depend on each other and I'm not sure if it would be effective to discuss all of them here. Is there any other way I could contact you, aside from GitHub (provided you're comfortable with the idea)?

[DimensionWarped]

I'm on SFGHQ's Discord if you want to reach out. And yeah, perfectly comfortable. My availability for the next few hours might be a tad lacking though.