-
Notifications
You must be signed in to change notification settings - Fork 9
SOS Index and Call Rate #71
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
base: main
Are you sure you want to change the base?
Conversation
- Wild View now shows 4 slots, can be manually scrolled through to see SOS data - Improved SOS Support. Select "Caller Slot", then "Ally Slot" will be dynamically determined. - Display Caller PP remaining to avoid struggle! - Display whether Adrenaline Orb effect is active. - Supports both SM/USUM. - Tested on UM hardware + retail cartridge.
Fix off-by-one in SOS ally slot
- Added Submenu with Capture for tracking correction value for SOS ally slot - Hold X and press Right on the dPad to set the caller slot to the current ally slot - Hold X and press Up/Down on the dPad to increment/decrement caller slot manually - When a new Caller slot is set, the current chain length is saved as state in order to determine future ally slots (correction value) - Updated README with new controls - Added coloration to Egg Ready, Masuda, and Charm text
Added is-wild switch to draw_pkx
|
Should probably squash this but not sure how to from WebUI |
zaksabeast
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably squash this but not sure how to from WebUI
I always merge + squash PRs into one commit to keep a clean history, so no worries about the number of commits. Pretty sure I've disabled the other merge settings to prevent accidents.
|
Converted to draft, found SOS Seed hook address completely dysfunctional for SM, need to debug |
- (hidden power and species are still in wild view)
serenagrace
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| [package] | ||
| name = "pokereader" | ||
| version = "0.8.0" | ||
| version = "0.8.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep this at 0.8.0 – it can be incremented later.
| } | ||
|
|
||
| pub fn draw_invalid_pkx() -> bool { | ||
| pnp::println!("No Data."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think showing a blank Pokemon might be more helpful.
Someone brand new to RNG who downloaded Reader for the first time and isn't sure if they set things up correctly might see "No Data." and be confused about if they did something wrong.
This message also doesn't have a way to self serve – there's no indication of how to fix the problem.
New people sometimes follow a YouTube video or a website tutorial with screenshots, and it's helpful if the video/images visually match what they're seeing.
Showing a blank PKX seems like it might be more intuitive for people.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree we can change the message itself, as "No Data" is a bit misleading... For Wild we can put "Not in Battle" or "Slot Empty" maybe? "Slot Empty" would also work well for Party, and "No Ally" would work for SOS.
Personally I believe that showing a blank Pkx really clutters the display when it doesn't need to-- especially for users who may X+Y lock the menu on Wild/SOS and getting in and out of several encounters. I agree that "No Data" is misleading, but I disagree that showing a blank Pkx is more intuitive, especially for things like cycling through party slots, it's a lot more convenient to see at a glance that there is nothing there.
| pnp::println!(color = RED, "Orb Not Active!"); | ||
| } | ||
|
|
||
| pnp::println!("RNG Frame: {}", main_rng.advances()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's call this "Advance", not "Frame".
We've spent years trying to get away from the word "Frame" because it confuses people. When new people ask what a frame is, the answer is "it's when the rng advances", and they seem to understand that much more.
"Frame" made sense in gen 3 when the video frames were tied to the RNG advancement, but not anymore.
PokeReader only uses "frame" in reference to video frames, and RNG uses show "Advance".
| pnp::println!("[X]/[Up]/[Down]:"); | ||
| pnp::println!(" - Swap Caller"); | ||
| pnp::println!(" Left/Right") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this still needed if we can auto-detect the Pokemon?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. We can't fit all the information about the caller and the ally on one screen-- therefore we simplify things and just show the caller's call rate and remaining PP, while the Ally needs to show ability, nature, IVs, PID, etc. the ability to swap whether the caller on the left or is on the right is still important because especially for high SOS chains we certainly cannot guarantee the caller will be on the right.
Another goal of mine is to fit everything on the one page to reduce motions through the Pokereader plugin, because they can be annoying. The user needing to constantly scroll around to see the caller and the ally on separate pages would be really bothersome, which is also why we need to read two Pkx per frame as I mentioned above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with showing everything on one screen if we can keep it performant for 2ds/o3ds users.
If manual inputs are annoying, we should find a way to auto-assign caller vs. ally, (e.g. a game hook or logic like checking the current caller's hp is 0).
Ideally the user can view the info without needing to manually assign themselves.
| // sgrace: I think it's better to panic if this overflows, | ||
| // As index > 624 should never naturally occur. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove this comment – the logic should prevents overflows.
| // sgrace: This one, however, can naturally overflow | ||
| // in Sfmt64 at index = 0, so handle that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the RNG is called several times per frame and needs to be performant, we should try to avoid the modulo.
Instead, let's add tests that will make sure we're not overflowing.
|
|
||
| #[derive(Clone, Copy, Debug, PartialEq, Eq)] | ||
| pub struct Sfmt { | ||
| pub struct Sfmt32 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add tests to make sure the RNG is producing expected random numbers? The main things to check are producing rands before and after the shuffle.
We'll want confidence this works as expected if it's modified in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the existing SFMT tests (now under Sfmt64) exercise this behavior, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's testing the u64s, but it'd still be good to make sure the u32s are expected in case we rewrite this in the future.
| pub fn reset(&mut self) -> usize { | ||
| self.counter.reset() | ||
| } | ||
| pub fn increment(&mut self) -> usize { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a new line above this please.
| self.counter.value() | ||
| } | ||
|
|
||
| pub fn _set(&mut self, value: usize) -> usize { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove this if it's unused.

No description provided.