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

Write the entirety of the Wiimote EEPROM, in a per-Wiimote file #8224

Open
wants to merge 1 commit into
base: master
from

Conversation

@Pokechu22
Copy link
Contributor

commented Jul 1, 2019

Previously, only Mii data was written. Additionally, the file containing mii data was shared for all Wiimotes, which made it a lot less useful.

Additionally, the file was read/written on each Wiimote read, even though the whole EEPROM was kept in memory. This was bad for performance and not particularly necessary (it did enforce that the data was properly shared between all Wiimotes, but that's not something I want).


Some notes about this:

  • If there is no Wiimote#.bin file, the previous mii.bin file will be read (if it exists) and used to populate the mii block. However, since the Wiimote#.bin files are not written until a write to EEPROM happens, this will continue happening on each startup. Also, the mii.bin file isn't ever deleted, which is probably fine but might get weird if all wiimotes have bin files with them since the mii file isn't used at that point.
  • I think it would be neat to have files not tied directly to Wiimote numbers, since you might want to save more data per game (or data from different games or whatever). But I've got no idea how to do that from a UI perspective.
  • It would be neat to import (and maybe export, though that may be dangerous) data to physical Wiimotes, but that does depend on having further management of the files than just tying it to the Wiimote number.
  • This includes writing EEPROM for the balance board, which is currently unknown. Technically Dolphin allowed reads and writes to it before, but it would only interact with the same mii.bin file. Now, a write to the EEPROM for the wii balance board will create a Wiimote5.bin file (I think), which might be incorrect. That said, this only happens after a write, which presumably no actual game does.
  • Data might be written too infrequently now. It's written when Reset() is called while the eeprom is dirty, which should include disconnects; however, that only happens when the game is closed eventually. If there's a crash or something in the middle, the file remains unwritten.
@mbc07

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2019

About exporting emulated Wiimote EEPROM to real Wiimote EEPROM: that's a bad idea as the user accessible area also store calibration data for the motion sensors and that's unique on each unit, overwriting these values will cause issues. If this get implemented, make sure to always skip the calibration data when exporting from emulated => real Wiimotes...

// Write out existing EEPROM
std::ofstream file;
File::OpenFStream(file, eeprom_file, std::ios::binary | std::ios::out);
file.write((char*)m_eeprom.data.data(), EEPROM_FREE_SIZE);

This comment has been minimized.

Copy link
@jordan-woyak

jordan-woyak Jul 2, 2019

Member

This should be a reinterpret_cast.

// Read existing EEPROM
std::ifstream file;
File::OpenFStream(file, eeprom_file, std::ios::binary | std::ios::in);
file.read((char*)m_eeprom.data.data(), EEPROM_FREE_SIZE);

This comment has been minimized.

Copy link
@jordan-woyak

jordan-woyak Jul 2, 2019

Member

reinterpret_cast

@@ -136,7 +172,7 @@ void Wiimote::Reset()
m_shake_state = {};
}

Wiimote::Wiimote(const unsigned int index) : m_index(index)
Wiimote::Wiimote(const unsigned int index) : m_index(index), m_eeprom_dirty(false)

This comment has been minimized.

Copy link
@jordan-woyak

jordan-woyak Jul 2, 2019

Member

You could just initialize this value in the class definition: bool m_eeprom_dirty = false;.

// Import from the existing mii.bin file, if present
std::ifstream file;
File::OpenFStream(file, mii_file, std::ios::binary | std::ios::in);
file.read((char*)m_eeprom.mii_data_1.data(), m_eeprom.mii_data_1.size());

This comment has been minimized.

Copy link
@jordan-woyak

jordan-woyak Jul 2, 2019

Member

reinterpret_cast

Write the entirety of the Wiimote EEPROM, in a per-Wiimote file
Previously, only Mii data was written.  Additionally, the file containing mii data was shared for all Wiimotes, which made it a lot less useful.

Additionally, the file was read/written on each Wiimote read, even though the whole EEPROM was kept in memory.  This was bad for performance and not particularly necessary (it did enforce that the data was properly shared between all Wiimotes, but that's not something I want).

@Pokechu22 Pokechu22 force-pushed the Pokechu22:wiimote-eeprom branch from ba1ef22 to d202abc Jul 2, 2019

@Pokechu22 Pokechu22 changed the title Write the entirety of the Wiimote EEPROM, in a per-Wiimotes file. Write the entirety of the Wiimote EEPROM, in a per-Wiimote file Jul 2, 2019

@Pokechu22

This comment has been minimized.

Copy link
Contributor Author

commented Jul 2, 2019

Another thing I just remembered: Save-states already did store the entirety of the EEPROM. However, loading a state generally wouldn't cause issues, as the mii data was re-read from disk each time so the loaded one was ignored. (The only case that could possibly happen is if it wrote without reading, which would only happen when saving on one of the transfer screens (e.g. sending miis to the remote). Now, since it loads on startup and saves on exit, a savestate will overwrite the previously stored data. I don't know if that's a problem (and I think that might already be the case for normal save files).

And regarding writing to a physical remote: Yeah, I'm aware of that. It would make sense to allow configuring which sections to write, with the Mii blocks and the user data block enabled by default and the others disabled with a warning on enabling them. Being able to chose to only write the user block seems useful, and making the rest also configurable would be nice.

@mbc07

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2019

And regarding writing to a physical remote: Yeah, I'm aware of that. It would make sense to allow configuring which sections to write, with the Mii blocks and the user data block enabled by default and the others disabled with a warning on enabling them. Being able to chose to only write the user block seems useful, and making the rest also configurable would be nice.

Honestly, (over)writing the calibration data should never be allowed if this get implemented. I already see users ignoring any warning and then complaining their Wiimotes are broken on the forums...

@Pokechu22

This comment has been minimized.

Copy link
Contributor Author

commented Jul 2, 2019

Honestly, (over)writing the calibration data should never be allowed if this get implemented. I already see users ignoring any warning and then complaining their Wiimotes are broken on the forums...

Fair enough. However, that's something that will be handled in a later PR; this one doesn't do anything with physical remotes. For now I think it's best to just not worry about it; when someone does something with physical remotes then we can worry about it. (I lack the hardware to actually do any testing with them in Dolphin).

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