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

Add fan levels auto-adjust #237

Merged
merged 2 commits into from
Apr 14, 2023
Merged

Add fan levels auto-adjust #237

merged 2 commits into from
Apr 14, 2023

Conversation

hungqbui
Copy link
Contributor

G-Helper has been very useful since I made the switch from Armoury Crate. But tweaking with the fan levels have been annoying because they don't auto adjust. This is my first open source contribution ever so I'm sorry if my implementation is very elementary. Also, if there are any GitHub etiquettes that I'm not aware of please let me know.

@manciuszz
Copy link

I also see O(log n) potential over the current O(n) in-efficient code. However, that's just food for thought.

@hungqbui
Copy link
Contributor Author

Yeah I definitely think there are way better ways to optimize it. The code itself isn't very DRY either. I'll try to figure it out, probably going to take me a while though. Thanks for the suggestion!

@manciuszz
Copy link

Yeah I definitely think there are way better ways to optimize it. The code itself isn't very DRY either. I'll try to figure it out, probably going to take me a while though. Thanks for the suggestion!

Benchmark. Maybe play around with a Binary Search Tree. Benchmark again. I doubt there's going to be a significant difference considering we are dealing with a very low amount of graph nodes (points). However, if you never used any data structures before, here's your chance.

@hungqbui
Copy link
Contributor Author

I haven't really learned Data Structures, at least not formally. But I'll look into using BST. I can't find a better implementation of this for an array.

@seerge
Copy link
Owner

seerge commented Apr 14, 2023

@hungqbui hello, thanks for contribution, I will check it :)

@hungqbui
Copy link
Contributor Author

@hungqbui hello, thanks for contribution, I will check it :)

You're welcome. It is definitely not the best implementation though, but I haven't found anything better at the moment.

@seerge seerge merged commit 4f9cc4a into seerge:main Apr 14, 2023
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.

3 participants