Skip to content

Invincibility stars fix#356

Open
QuartzTS wants to merge 7 commits into
Techokami:mainfrom
QuartzTS:main
Open

Invincibility stars fix#356
QuartzTS wants to merge 7 commits into
Techokami:mainfrom
QuartzTS:main

Conversation

@QuartzTS

Copy link
Copy Markdown

Trying to fix the invincibility stars, nothing else to say here for now..

This fixes the scene leak caused by freeing the child node instead of the root node of the RotatingParticle scene for now, ima start with experimenting with the new invincibility stars thingy later..
@DimensionWarped

Copy link
Copy Markdown
Collaborator

I assume by the fact that this is a draft that you aren't ready for review yet?

@QuartzTS

Copy link
Copy Markdown
Author

I know, but like, should I open it for testing the fix or keep it as a draft till I do smth with the invincibility stars? (Forgot to say this is my first time opening a pull request and contributing to another repository..)

@DimensionWarped

Copy link
Copy Markdown
Collaborator

Oh, generally speaking you wouldn't create a PR until you have some kind of fix actually ready unless you were specifically reaching out to a dev for help which is usually what the drafts are for... that or sharing a fix that you aren't adequately sure of and don't mind tossing over the fence should you disappear for someone else to look at.

@QuartzTS

Copy link
Copy Markdown
Author

Well, I just fixed the scene leak, but I'm also trying to implement the invincibility star effect from Sonic 2, even tho it feels like I might need help at this or smth.. maybe no problem with opening the pull request for testing

@QuartzTS
QuartzTS marked this pull request as ready for review June 16, 2026 08:19
@DimensionWarped

Copy link
Copy Markdown
Collaborator

I tried to eyeball an understanding of what happens with the Sonic 2 invincibility star effect, but it's kinda busy so it's hard to tell the exact details of it. I'll try to work with you on it soon.

@Caverns4

Copy link
Copy Markdown
Collaborator

From what I can infer, its actually not as complicated as it looks at least on an outer level. It looks like there are four pairs of stars, each one orbiting Sonic with each one beeing offset by where Sonic was a certain number of frames ago, and then there's a scond part to the efect that essentially just spawns stars at a psuedo-random offset at different set intervals.

@QuartzTS

Copy link
Copy Markdown
Author

Huh, I knew there is more to it.. anyways, here's what I've done so far, It's just 8 rotating particles spinning around the player's position in whatever direction it's in, I've deleted the two star generator nodes and replaced them with rotating particle scenes and tweaked some code, I know it's not much progress and there's nothing much to it, I just need to figure out the animations and how to make it follow the player and other stuff..

2026-06-16.21-34-51.mp4

@QuartzTS

QuartzTS commented Jun 22, 2026

Copy link
Copy Markdown
Author

Okay, so I guess the invincibility stars in Sonic 2 consists of 4 pairs of stars like @Caverns4 said, and they're spinning around the player in whatever direction the player is facing, with the main pair being locked into the player's position while the other pairs trail behind the player (kinda like afterimages or smth idk), and the rotation and animation of the main pair are faster than the trail pairs, so with this, I brought back the two star generators like normal so that they function like a main pair, even tho I still feel like it shouldn't be in the player scene, and I still gotta figure out how to make the trail pairs by instantiating them when the invincibility monitor is destroyed and freeing them when the invincibility effect is finished..

2026-06-22.20-15-57.mp4

Btw, all this made me rethink of the particle system, since I initially asked for a particle rework cuz I thought having multiple particle objects is a bit redundant, and now idk what to do since the invincibility stars doesn't function like a generic particle.. (btw, I guess the invincibility stars effect is the only thing that uses RotatingParticles, so I deleted every other animation in the RotatingParticle scene and kept the invincibility animation, and did the same for the player's star generators)

QuartzTS added 4 commits July 15, 2026 07:26
After days of trial and error ~~and procrastination~~, I've finally figured it out, still needs documentation tho, and maybe a bit more cleanup, idk.. anyways, ima rest a bit before attempting at doing the particle system
@Daniel-Cortez

Daniel-Cortez commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

I left a few suggestions about how the code could be improved for a bit. You probably won't see them for now, as they need to be approved by the maintainers first, but they are nothing major, just a few ideas for performance optimizations and code safety.
(Or maybe the maintainers won't even approve them - which would be okay too, because again, those suggestions are mostly just tiny nitpicks.)
Here's a screenshot of one of them so you could get the idea:
unnamed

In any case, it's always nice to see a new face among the contributors! 👍

Corrected the star angles and adjusted the pair offsets (I hope this is the last adjustment or smth..)
@DimensionWarped

DimensionWarped commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

I'm fine with using a Array[PackedByteArray] for storage of byte data specifically. I'm not as sure about Vector2s. I do think the array should be typed if it's easy to do so, but I'm forgetting a bit about how cleanly array typing works in 4.3

EDIT: Oh, but there's a PackedVector2Array that you can probably use there instead. I reviewed the function of Packed Arrays in general and I'm inclined to say yeah, use them. They make sense. I doubt it's going to be an optimization we really need or anything, but it's fine.

@QuartzTS

Copy link
Copy Markdown
Author

So, I've converted all arrays into packed array types, and I only have a problem with the arrays inside the frame array, for whatever reason, the game just throws a lot of errors saying "index [insert a somehow big number] out of range" or smth.. (also I wanna see the suggestions, honestly lol..)

@DimensionWarped

Copy link
Copy Markdown
Collaborator

a somehow big number

I assume you are providing the number somehow right? Got a code snippet?

@QuartzTS

QuartzTS commented Jul 16, 2026

Copy link
Copy Markdown
Author

Here are some snippets of code from the script:

const STAR_FRAME_ARR: Array[PackedByteArray] = [
	[7, 4, 6, 4, 4, 6, 4, 7, 4, 6, 6, 4],
	[2, 3, 4, 5, 6, 7, 6, 5, 4, 3],
	[1, 2, 3, 4, 5, 6, 7, 6, 5, 4, 3, 2],
	[0, 1, 2, 3, 4, 5, 6, 5, 4, 3, 2, 1],
]
		var frame_arr: PackedByteArray = STAR_FRAME_ARR[star_pair_num] # The errors won't pop up if you replace PackedByteArray with Array, since the packed array gets converted into a regular array
		star.frame = frame_arr[(star_frame[frame_index] + frame_increment) % frame_arr.size()] # The errors point to this line cuz the numbers in the packed byte arrays (I even tried making it an int32 array but it didn't work) are out of range, presumably cuz the numbers are encoded or smth, idk..

Edit: Wait, setting them to packed byte arrays don't throw errors and the stars don't animate, while setting them to int32 arrays instead of byte arrays throws these errors? Also I printed the sizes of the packed arrays and they were somehow incorrect (in byte arrays case, it returns 0, and in int32 arrays case it returns 24 for whatever reason, idk what's going on..)

@QuartzTS

Copy link
Copy Markdown
Author

Nevermind, solved this issue, I just had to construct the arrays with the PackedByteArray() constructor, even tho it needs the array that stores them to be a normal variable and not a constant (btw, still waiting for the suggestions..)

@Daniel-Cortez

Daniel-Cortez commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

I only meant using Array[PackedByteArray] on that Array[Array] line (or you may even want to use PackedVector2Array instead, as @DimensionWarped suggested). I would never recommend replacing all int arrays with packed ones.

Regular arrays can be initialized with array literals, e.g. ([ 0, 1, 2, 3 ]), which are compile-time expressions (which means there's no runtime cost for array initialization). But, as you've already noticed, there's no distinct syntax for packed array literals - instead you have to initialize them at runtime with a regular array using a constructor (e.g. var a: PackedByteArray = PackedByteArray([ 0, 1, 2, 3 ])), which means there's a runtime cost for 1) memory allocation for the packed array, and 2) copying of the elements from the said regular array literal (and this is why you had to replace const with var at the declaration of STAR_FRAME_ARR, as you mentioned earlier). In case of replacing Array[Array] with Array[PackedByteArray] this can be justified: you're trading off an extra cost for a one-time initialization of a packed array for a more effective bytecode when reading/modifying the elements of the said array later, as strict typing means no extra type checks when accessing the array elements. But replacing all regular Array[int] arrays with PackedByteArray/PackedInt32Array/PackedInt64Array is definitely not worth it - the elements of Array[int] are already strictly typed, so there would be no extra code safety, no performance boost, and it would only complicate your code.

TL;DR: Packed arrays are not a silver bullet, and you should not replace all regular arrays with them.

@QuartzTS

Copy link
Copy Markdown
Author

Wait, is it a good idea if I kept it as Array[Array] and converted it to a packed array for the local variable frame_arr inside the for loop below?

@Daniel-Cortez

Daniel-Cortez commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Ah, right, you're caching a nested packed array from STAR_FRAME_ARR into frame_arr. Yes, you need to use PackedByteArray for that one too. For some reason I initially thought you were going to replace all arrays with packed ones. If that's not the case, then my bad for misunderstanding you, please disregard my previous post.

@QuartzTS

Copy link
Copy Markdown
Author

Well, I DID wanna replace all arrays with packed ones, but I've changed my mind when you told me not to, but then I asked if I could keep the nested arrays untyped and convert them into packed arrays in the frame_arr local variable, so no problem, bro..

A few adjustments and wip documentation (still waiting for the suggestions, btw..)
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.

4 participants