Add 4-bit Two’s Complement mode #9
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new 4-bit Two's Complement game mode to the binary numbers puzzle game. The mode displays suggestions as signed values (-8 to +7) while maintaining internal raw bit patterns for correctness.
- Added
FourTwosComplementenum variant to theBitsenum - Implemented signed value conversion for display using
raw_to_signed()method - Modified puzzle generation logic to handle raw bit patterns for two's complement mode
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/app.rs | Added menu entry for the new Two's Complement mode |
| src/binary_numbers.rs | Core implementation including enum variant, conversion logic, suggestion generation, and display formatting |
| main.diff | Duplicate diff file (appears to be a generated artifact) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/binary_numbers.rs
Outdated
| pub const fn to_int(&self) -> u32 { | ||
| match self { | ||
| Self::Four | Self::FourShift4 | Self::FourShift8 | Self::FourShift12 => 4, | ||
| Self::Four | Self::FourShift4 | Self::FourShift8 | Self::FourShift12 | Self::FourTwosComplement=> 4, |
There was a problem hiding this comment.
Missing space before | Self::FourTwosComplement. There are two spaces before the pipe before FourTwosComplement and no space after the pipe before the arrow. Should be: Self::FourShift12 | Self::FourTwosComplement => 4,
| Self::Four | Self::FourShift4 | Self::FourShift8 | Self::FourShift12 | Self::FourTwosComplement=> 4, | |
| Self::Four | Self::FourShift4 | Self::FourShift8 | Self::FourShift12 | Self::FourTwosComplement => 4, |
| suggestions.push(num); | ||
|
|
||
| if bits.is_twos_complement() { | ||
| // For two's complement, generate unique raw bit patterns (0-15) |
There was a problem hiding this comment.
[nitpick] Comment states range is (0-15) but for 4-bit values the range should be (0..16), which generates values 0-15 inclusive. The comment is technically correct but could be clearer by stating '0 to 15 inclusive' or matching the Rust range syntax '0..16'.
| // For two's complement, generate unique raw bit patterns (0-15) | |
| // For two's complement, generate unique raw bit patterns in the range 0..16 (i.e., 0 to 15 inclusive) |
| if bits.is_twos_complement() { | ||
| // For two's complement, generate unique raw bit patterns (0-15) | ||
| let mut raw_values: Vec<u32> = Vec::new(); | ||
| while raw_values.len() < bits.suggestion_count() { | ||
| let raw = rng.random_range(0..u32::pow(2, bits.to_int())); | ||
| if !raw_values.contains(&raw) { | ||
| raw_values.push(raw); | ||
| } | ||
| } | ||
| // Store raw bit patterns directly | ||
| suggestions = raw_values; | ||
| } else { | ||
| // For unsigned modes | ||
| while suggestions.len() < bits.suggestion_count() { | ||
| let raw = rng.random_range(0..u32::pow(2, bits.to_int())); | ||
| let num = raw * scale; | ||
| if !suggestions.contains(&num) { | ||
| suggestions.push(num); | ||
| } | ||
| } |
There was a problem hiding this comment.
[nitpick] The two's complement branch duplicates the same logic pattern as the else branch (lines 771-777) with only the scaling difference. Consider refactoring to eliminate duplication, such as: generate raw_values first, then apply scaling conditionally when building suggestions.
| if bits.is_twos_complement() { | |
| // For two's complement, generate unique raw bit patterns (0-15) | |
| let mut raw_values: Vec<u32> = Vec::new(); | |
| while raw_values.len() < bits.suggestion_count() { | |
| let raw = rng.random_range(0..u32::pow(2, bits.to_int())); | |
| if !raw_values.contains(&raw) { | |
| raw_values.push(raw); | |
| } | |
| } | |
| // Store raw bit patterns directly | |
| suggestions = raw_values; | |
| } else { | |
| // For unsigned modes | |
| while suggestions.len() < bits.suggestion_count() { | |
| let raw = rng.random_range(0..u32::pow(2, bits.to_int())); | |
| let num = raw * scale; | |
| if !suggestions.contains(&num) { | |
| suggestions.push(num); | |
| } | |
| } | |
| // Generate unique raw bit patterns for suggestions | |
| let mut raw_values: Vec<u32> = Vec::new(); | |
| while raw_values.len() < bits.suggestion_count() { | |
| let raw = rng.random_range(0..u32::pow(2, bits.to_int())); | |
| if !raw_values.contains(&raw) { | |
| raw_values.push(raw); | |
| } | |
| } | |
| // Apply scaling if not two's complement | |
| if bits.is_twos_complement() { | |
| suggestions = raw_values.clone(); | |
| } else { | |
| suggestions = raw_values.iter().map(|raw| raw * scale).collect(); |
|
|
||
| let current_number = suggestions[0]; // scaled value | ||
| let raw_current_number = current_number / scale; // back-calculate raw bits | ||
| let current_number = suggestions[0]; // scaled value or raw for twos complement |
There was a problem hiding this comment.
Corrected spelling of 'twos' to 'two's' in comment for consistency with the mode name.
| let current_number = suggestions[0]; // scaled value or raw for twos complement | |
| let current_number = suggestions[0]; // scaled value or raw for two's complement |
| pub const fn raw_to_signed(&self, raw: u32) -> i32 { | ||
| match self { | ||
| Self::FourTwosComplement => { | ||
| // 4-bit two's complement: range -8 to +7 | ||
| if raw >= 8 { | ||
| (raw as i32) - 16 | ||
| } else { | ||
| raw as i32 | ||
| } | ||
| }, | ||
| _ => raw as i32, // other modes use unsigned | ||
| } | ||
| } |
There was a problem hiding this comment.
The new raw_to_signed method lacks test coverage. Given that the repository has comprehensive tests for other Bits methods (see bits_properties test), this conversion logic should be tested to verify correct two's complement conversion (e.g., 8 → -8, 15 → -1, 7 → 7, 0 → 0).
| pub const fn is_twos_complement(&self) -> bool { | ||
| matches!(self, Self::FourTwosComplement) | ||
| } |
There was a problem hiding this comment.
The new is_twos_complement helper method lacks test coverage. Consider adding a test to verify it returns true only for FourTwosComplement and false for all other bit modes.
|
Hi @lucaspalomodevelop, thanks for the contribution! I have only seen the PR today, I will have a closer look after work. Already got some ideas how to improve the menu and apply signed/unsigned mode to all game modes. Please run |
|
@lucaspalomodevelop as you've probably seen, my latest changes have largely overriden your work (while keeping the feature). that is because I had a menu overhaul in the works, but also I wanted to integrate signed mode deeper. You can now toggle signed mode by pressing left/right in the menu. Please let me know (or do another PR) if something behaves differently than expected! And thanks again for the contribution :) |
|
The solution is much more elegant and exactly what I had in mind, thank you very much! |
2 participants
Summary
Adds a new 4-bit Two’s Complement game mode.
Displays suggestions as signed values (-8..+7) while keeping internal raw bit patterns for correctness.
User-visible changes
New start menu entry: “easy Two’s complement (4 bits)”.
In this mode, suggestion numbers are shown as signed values (e.g., 1111 → -1, 1000 → -8, 0111 → 7).
Separate high score bucket for this mode.
Rationale
Two’s complement is the most common signed representation; adding it helps players practice reading signed values without changing the core puzzle flow.
Keeping raw bit patterns internally avoids ambiguity and preserves correctness when mapping to signed display values.
Compatibility
No breaking changes to existing modes.
High scores for the new mode are isolated via key 42.