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

feat: KITT usermod effect #3756

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

casesolved-co-uk
Copy link

v2 usermod for replicating the effect found on KITT, the car from the 80's TV show Knightrider.
The original effect was in Red, however, the usermod also supports palettes

Has 4 controls:

  • Speed
  • Tail length
  • Delay before start
  • Dual

Similar to Sinelon but uses triangular wave not sine.

@blazoncek
Copy link
Collaborator

That seems similar to K.I.T.T (Scanner and Scanner dual) existing effects.

@casesolved-co-uk
Copy link
Author

That seems similar to K.I.T.T (Scanner and Scanner dual) existing effects.

Yes, but there is no delay feature and this combines two effects into one

@blazoncek
Copy link
Collaborator

Yes, but there is no delay feature and this combines two effects into one

I managed to test the effect but apart from the delay on one side of the scan it is no different than the mentioned Scanner.
If you do want "delay" why not on both sides of the scan? I admit this might be personal preference.

I was also wondering about modulus (%) used in the effect. It is quite inefficient.

What if we incorporate the delay into original effect?

@casesolved-co-uk
Copy link
Author

In the original show it was quite common to see intermittent scans, but the scan always did a full loop not one way. Plus the effect gets quite tiring if it is continuous.

Optimisation vs maintenance. A single division operation per segment once every 23ms? How many microseconds are saved, 0.01us?

I'd have thought changing the behaviour of an existing effect would be the last thing to suggest for backward compatibility.

@softhack007
Copy link
Collaborator

Can we close this PR in favour of #3763 ? Or is this usermod still "sufficiently different" so to keep it open?

@casesolved-co-uk
Copy link
Author

Can we close this PR in favour of #3763 ? Or is this usermod still "sufficiently different" so to keep it open?

I was going to add support for 2D matrices but since blaz doesn't seem to like collaboration maybe he can add it to his. I don't have time.

@blazoncek
Copy link
Collaborator

blaz doesn't seem to like collaboration

I did not know this. Thanks for enlightening me.

@softhack007 softhack007 added the rebase needed This PR needs to be re-based to the current development branch label Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effect rebase needed This PR needs to be re-based to the current development branch usermod usermod related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants