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

WiimoteEmu: Add option to use system battery level #8329

Open
wants to merge 1 commit into
base: master
from

Conversation

@blaahaj
Copy link

commented Aug 23, 2019

Only supports Windows and Mac right now.

#endif
else
{
m_status.battery = u8(std::lround(m_battery_setting.GetValue() / 100 * MAX_BATTERY_LEVEL));

This comment has been minimized.

Copy link
@blaahaj

blaahaj Aug 23, 2019

Author

Note: I have left this code as it is, but I think it is wrong. MAX_BATTERY_LEVEL is way beyond 100% if we use HOME menu as a reference, so even if you set the m_battery_setting to 50% currently, it still shows up as 4/4 bars.

This comment has been minimized.

Copy link
@jordan-woyak

jordan-woyak Aug 23, 2019

Member

The Wii Remote hardware definitely produces values around 0xc8 (decimal 200) for new batteries.

Perhaps the HOME menu has been designed to show 4/4 bars for partially drained batteries to comfort players. 4/4 is more of a "you're all good" rather than a "it rounds up to 100%".

Having said that, I don't know the best way to implement this feature when the visual does not match the underlying value nicely.

This comment has been minimized.

Copy link
@blaahaj

blaahaj Aug 24, 2019

Author

I think I will check some other Wii software that displays the battery, to see if they use the value differently.

This comment has been minimized.

Copy link
@altimumdelta

altimumdelta Aug 24, 2019

Contributor

It may be designed to show a visual change only when the level comes below the standardized face value of the required 1.5V AA batteries, which gives a number of 3.0, but these batteries usually come at 1.65V when new/fully charged and that would produce a total of 3.3V, so maybe they consider the 0.3V over the 3.0 as something extra (The Wii Remote IR camera is labeled as 3.3V so the max could actually be the optimal value for the whole thing)

For example it could be something like:

  • 200 = 3.3 VDC
  • 100 = 3.0 VDC
  • 0 = 2.7 VDC

This comment has been minimized.

Copy link
@blaahaj

blaahaj Aug 24, 2019

Author

I tried Metroid Prime 3 and it seems to display the battery value bars exactly the same as the HOME menu, at least for 100, 75, 51 and 50.

You might be onto something with the voltage thing. Another option would be for the settings to have 200% as the maximum and default value, corresponding to the integer 200.

This comment has been minimized.

Copy link
@jordan-woyak

jordan-woyak Aug 24, 2019

Member

I don't like the "hacky math".

I think it would make more sense to map the system's 0-100% level directly to the 0-200 value produced by a Wii Remote.

Why must we hack the value attempting to produce linear output on the "4 bars" when that is not even how the real system works?

Yes, we are going to get "4/4" even at 50% but that is how the real Wii Remote works.

This comment has been minimized.

Copy link
@blaahaj

blaahaj Aug 24, 2019

Author

I think it would make more sense to map the system's 0-100% level directly to the 0-200 value produced by a Wii Remote.

Well, yes, you could do that, but it would also make the feature completely useless.

This comment has been minimized.

Copy link
@blaahaj

blaahaj Aug 24, 2019

Author

Why must we hack the value attempting to produce linear output on the "4 bars" when that is not even how the real system works?

Battery voltage doesn't decline linearly with battery capacity. I should probably be trying to emulate that behaviour rather than doing this fudge though.

This comment has been minimized.

Copy link
@jordan-woyak

jordan-woyak Aug 24, 2019

Member

I'd be happier with emulating a battery voltage decline. And with that, maybe even the UI controlled battery level should use that to produce a 0-200 value.

This comment has been minimized.

Copy link
@blaahaj

blaahaj Aug 24, 2019

Author

I looked up an example AA battery voltage curve and made a function that roughly fit it, but it is no better than the identity function in terms of making a given percentage produce a sensible UI result. Now I am wondering what the Wiimote byte actually means, and how the HOME Menu interprets it.

@blaahaj blaahaj force-pushed the blaahaj:battery branch from fbcccb9 to 0e10bdf Aug 23, 2019

Source/Core/Core/HW/WiimoteEmu/EmuSubroutines.cpp Outdated Show resolved Hide resolved
if (Core::WantsDeterminism())
{
// One less thing to break determinism:
m_status.battery = MAX_BATTERY_LEVEL;
}
#ifdef SYSTEM_BATTERY_LEVEL_AVAILABLE
else if (m_system_battery_setting.GetValue() && (battery = GetSystemBatteryLevel()))

This comment has been minimized.

Copy link
@Warepire

Warepire Aug 24, 2019

While assigning values in if-statements is perfectly legal, it's prone to make the code harder to maintain properly as the intention of the code is not as explicit as it can be.

This comment has been minimized.

Copy link
@blaahaj

blaahaj Aug 24, 2019

Author

Yes, but it is most elegant if written this way. I don't like that it would otherwise fetch the system battery even if you turned that feature off. Alternatively, I would have to add another m_system_battery_setting check.

Source/Core/Core/HW/WiimoteEmu/SystemBattery.cpp Outdated Show resolved Hide resolved

@blaahaj blaahaj force-pushed the blaahaj:battery branch 3 times, most recently from de783b2 to d10d971 Aug 24, 2019

WiimoteEmu: Add option to use system battery level
Only supports Windows and Mac right now.

@blaahaj blaahaj force-pushed the blaahaj:battery branch from d10d971 to 42c1aef Aug 25, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.