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

Refactor for clearer and more performant code #466

Merged
merged 8 commits into from
Feb 19, 2024

Conversation

Shute052
Copy link
Collaborator

@Shute052 Shute052 commented Feb 19, 2024

Simplified PR #463 to refactor for clearer and more performant code.

Changelog

Logic Refactoring

  • Refactored DeadZoneShape::cross_deadzone_value() and DeadZoneShape::ellipse_deadzone_value() to improve clarity of logic.
  • Refactored InputStreams::button_pressed() and InputStreams::input_value() functions for both MouseWheel and MouseMotion events by using more efficient f64 accumulation, reducing unnecessary match.

Deduplication

  • Extracted ActionState::action_data_mut_or_default() function to remove duplication in the parts checking press(), release(), and consume().
  • Extracted deadzone_axis_value() to remove duplication in the parts applying deadzone to input values.
  • Reused the existing virtual_axis_button_clash() for checking clashes of VirtualAxis.
  • Extracted send_keyboard_input(), send_mouse_wheel(), send_mouse_motion(), and send_gamepad_button_changed() for MutableInputStreams when mocking input.
  • Extracted collect_events_cloned() to remove duplication in the parts collecting the cloned events of MouseWheel and MouseMotion.
  • Reused the existing InputStreams::extract_dual_axis_data() to check InputKind::DualAxis has been button_pressed().
  • Extracted get_gamepad_value() closure to reuse the logic getting values from InputStreams::gamepad_axes and InputStreams::gamepad_buttons_axes.
  • Extracted InputStreams::extract_single_axis_data() for computing the input_value() of VirtualAxis and the input_axis_pair() of VirtualDpad.
  • Extracted RawInputs::merge_input_data() to remove duplication in the parts merging UserInputs into a RawInputs.

Code Quality

  • Used the existing std::consts like FRAC_PI_2 rather than magic numbers.
  • Used f32::from() rather than manual if when just getting a f32 from bool value.
  • Used matches! rather than manual match when just getting a bool field value inside Option.
  • Used if let Some() rather than manual match when the None arm does nothing.
  • Used Result::ok() rather than manual match when casting Result into Option.
  • Used Option::then() rather than manual match when just mapping its value.
  • Used bool::then_some() rather than manual if when just mapping it into Option.
  • Used iterators rather than manual for loops when the logic can be replaced with simple and descriptive operators.
  • Used Itertools::join() rather than manual for loop when just converting values into a string.
  • Used Option::into_iter().chain(Iter) rather than manual if for identical behavior, as they result in the same assembly code.
  • Used HashMap::entry().or_default() rather than manual HashMap::raw_entry_mut().or_insert_with() to ensure its value existence.
  • Used assert_eq! rather than assert! when asserting two values are equal.

Typo

  • Fixed typo in the documentation of DeadZoneShape.

Documentation

  • Added missing documentation of UserInput::len() for VirtualAxis.

@Shute052 Shute052 marked this pull request as ready for review February 19, 2024 06:44
@Shute052
Copy link
Collaborator Author

Ready for review, with updated changelog in the description.

@alice-i-cecile alice-i-cecile added the code-quality Make the code faster or prettier label Feb 19, 2024
@alice-i-cecile alice-i-cecile merged commit 8bf7c61 into Leafwing-Studios:main Feb 19, 2024
4 checks passed
@alice-i-cecile
Copy link
Contributor

Awesome, thank you! That should make the migration PR much clearer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-quality Make the code faster or prettier
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants