Skip to content

Conversation

@0x5066
Copy link
Contributor

@0x5066 0x5066 commented Nov 29, 2025

Shouldn't have happened in the first place...

@netlify
Copy link

netlify bot commented Nov 29, 2025

Deploy Preview for tourmaline-kringle-c98715 canceled.

Name Link
🔨 Latest commit 7754a51
🔍 Latest deploy log https://app.netlify.com/projects/tourmaline-kringle-c98715/deploys/692a435ce15a7b0008442a42

@captbaritone
Copy link
Owner

Not super happy about adding a magic number here. I think we need to wait until we can find a proper solution.

Overall, I think we need to get to a place where the logic in the visualizer here has some clearer justification or shared understanding about why it works the way it does. I'm starting to ask myself the question: If I ever want to revisit/refactor this code, how will I know if I've regressed it or made it better?

We need some documented process describing how we picked the logic we have. Was it inspired by some other implementation? Do we have some manual process we can replicate to guess/check what these various numbers should be? Do we have a way to objectively assess what we have so that multiple people can collaborate? Or is the plan just "fiddle with the numbers and try it out side by side with Winamp a bit and if it looks better, ship it!". That feels hard to sustain over time.

@0x5066
Copy link
Contributor Author

0x5066 commented Dec 2, 2025

I feel as though it'd be better to rewrite this whole thing from scratch, sure, from the outside it does the part of looking like the thing, but to be honest with ourselves; This can be done better.

I could pop open the hood in Ghidra and see what I can gleam from it, but then it's possible that we're entering a gray area with regard to reimplementing it, unless it's made clear through notes that only an older copy of Winamp was looked at, or we can still stick with the approximations based on observing visual behavior.

Perhaps I'm thinking too hard about this, but rewriting this whole file has been on my mind for a while now.

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.

2 participants