Copy Wii save for current game for Netplay and TAS #4546

RisingFog · 2016-12-24T01:40:13Z

This brings back functionality prior to the "Blank NAND" to use saves in Netplay and TASes.

I would like some testing and comments on if this is the proper way to go about this.

JMC47 · 2016-12-24T05:29:52Z

I was able to sync up with saves. Have not tested writing + reloading though.

We need an option to disable this in the Netplay Window for smash users though.

JMC47 · 2016-12-24T05:56:06Z

@E2xD Can you get netplay people to test this out? Should allow loading Wii saves in netplay; assuming both players have the save. I don't know if it's useful for smash (in fact, it's probably detrimental) but it'l be important for other games. And we have to make sure we don't break Brawl netplay in the process.

Source/Core/Core/Movie.cpp

+  {
+    // TODO: Check for the actual save data
+    if (File::Exists(user_save_path + "banner.bin"))
+      SetClearSave(false);


Source/Core/Core/Movie.cpp

+
+void ShutdownWiiRoot()
+{
+  if (!s_temp_wii_root.empty())


Source/Core/Common/NandPaths.cpp

@@ -54,6 +56,21 @@ void ShutdownWiiRoot()
 {
  if (!s_temp_wii_root.empty())
  {
+    u64 tmd_title_id = Movie::GetTitleId();


Source/Core/Common/NandPaths.cpp

+      // Backup the existing save just in case it's still needed.
+      if (File::Exists(user_save_path + "banner.bin"))
+      {
+        File::CopyDir(user_save_path, user_save_path + "../backup/");


JosJuice · 2016-12-24T09:46:31Z

Why is the second commit an improvement? The temporary NAND handling isn't only used for movies, it's used for netplay too.

RisingFog · 2016-12-24T17:45:26Z

@JosJuice It's an improvement because GCC had linking issues when trying to use GetTitleId() from within NandPaths in the first commit, and it was the best place I could think of putting it to resolve the issues.

JMC47 · 2017-01-08T13:45:50Z

This pull request works perfectly. Tested with Dokapon Kingdom across 2 days and 3 sessions so far.

We probably need to have a way to let the Brawl players avoid using this though...

JosJuice · 2017-01-08T14:30:58Z

Instead of putting code in Movie.cpp that doesn't fit there, could you put it in the new WiiRoot.cpp file created by PR #4621? (I suppose this means you would need to wait for that PR, though)

JosJuice · 2017-01-08T15:53:04Z

I've attempted to improve the title ID handling that I commented about earlier: PR #4629

Source/Core/Core/Movie.cpp

+  if (IsRecordingInput())
+  {
+    // TODO: Check for the actual save data
+    SetClearSave(File::Exists(user_save_path + "banner.bin"));


Source/Core/Core/Movie.cpp

+    SetClearSave(File::Exists(user_save_path + "banner.bin"));
+  }
+
+  if (Core::g_want_determinism && !IsStartingFromClearSave())


JMC47 · 2017-01-10T11:21:04Z

Considering netplay users usually stay behind a bit. I think we can merge this and someone else do save syncing so that there aren't more confusing options.

@Helios747 @leoetlino @JosJuice @mimimi085181 is that a reasonable solution?

JosJuice

@JMC47 Sure, but we shouldn't merge this before the problems I pointed out are fixed. (IsStartingFromClearSave being expected to be correct when Movie isn't active, Movie.cpp containing code that should be somewhere else, maybe the title ID mess too (though it could be argued that all the title ID problems existed before this PR and can be fixed later by PR #4629))

JosJuice · 2017-01-14T19:34:16Z

I did some more thinking about the /backup/ folder stuff, and I realized that this PR makes it more problematic. Before, /backup/ used to get deleted on the next non-Movie run, but now it seems like it will stay there forever unless the user manually goes poking around in the NAND. Do we really want that?

And do we actually need a /backup/ folder at all? It seems like the previous code just used it as a place to keep the "actual" save while running a movie, but we don't need that anymore since we have two entirely separate NANDs now. Though I suppose the user might not want to overwrite their actual save with one from a TAS... And we can't just hardcode it to not keep the save from a deterministic session, because people generally do want to keep their saves from netplay. It seems like a tricky situation.

E2xD · 2017-01-18T06:52:12Z

I'm sorry. Ive been away from the dolphin netplay scene for some time (Life/Holidays/Work/Etc). Just to make sure I am clear, you want to add NAND syncing at the risk of compromising netplayers? Why not make it a selectable option under Config > Wii? Like have a toggle saying "Blank NAND" or something. Just my 2 cents.

JosJuice · 2017-01-18T06:57:38Z

This commit does not add NAND syncing. In other words, no NAND files will be send over the network to other players. What it does do is copy the save files for the current game from the user's real NAND to the empty temporary NAND. I suppose SSBB/PM players don't want that since they want to have no save files when using netplay, right?

E2xD · 2017-01-18T07:04:21Z

Correct.

JMC47 · 2017-01-18T07:39:35Z

Netplayers usually don't update builds often and this is a stepping stone toward more and better netplay features. My idea for Brawl was to change the behavior of write to memcards and SD cards to also control nand access. Unchecked would mean no save import or writing, checked would mean it would import and write back saves.

…

On Wed, Jan 18, 2017, 2:04 AM E2xD ***@***.***> wrote: Correct. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4546 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGSuQShWomORQc_Fxs3ncDP-3w998ka2ks5rTbl6gaJpZM4LVIks> .

Source/Core/Core/Movie.cpp

+
+static std::string s_temp_wii_root;
+
+void InitializeWiiRoot(bool use_dummy)


Source/Core/Core/Movie.cpp

@@ -1580,4 +1580,77 @@ void Shutdown()
  tmpInput = nullptr;
  tmpInputAllocated = 0;
 }
+
+void CheckWiiSaves()


The issues I commented on earlier are (mostly) fixed

JosJuice · 2017-01-18T16:49:27Z

Only two of my comments are relevant anymore, and I'm not sure if they should block this PR from being merged or not. If others think it's fine to merge the affected parts as-is, you have my LGTM. The two potential blockers are:

~~Should we really pollute the NAND with backup folders?~~ EDIT: The PR doesn't put things like that in the NAND anymore. It still puts them somewhere, and I'm not completely sure if that's the best way to go, but I don't consider this to be a blocker.
The pre-existing title ID handling is terrible, but it's improved by PR Don't call Movie::SetTitleId from ES #4629. I was thinking that we should clean it up (by merging PR Don't call Movie::SetTitleId from ES #4629) before merging this code that really makes use of Movie::GetTitleId, but I guess it could be fine to just merge that other PR later. EDIT: PR Don't call Movie::SetTitleId from ES #4629 has been merged.

JosJuice · 2017-01-18T17:03:39Z

Sorry, there's another problem: The commit history is messy. Various fixups have been applied to the last commit, and I don't think all of them belong in that commit. In particular, most of the second commit gets reverted by the last commit (instead of simply not being commited to begin with). Also, the first commit probably shouldn't mark RootUserPath as static (it's reverted later).

EDIT: All the commits have been squashed.

Source/Core/Core/WiiRoot.cpp

+        Common::GetTitleDataPath(Movie::GetTitleId(), Common::FROM_CONFIGURED_ROOT);
+    std::string user_backup_path =
+        File::GetUserPath(D_BACKUP_IDX) +
+        StringFromFormat("%08x/%08x/", (u32)(Movie::GetTitleId() >> 32), (u32)Movie::GetTitleId());


Source/Core/Core/WiiRoot.cpp

+        Common::GetTitleDataPath(Movie::GetTitleId(), Common::FROM_SESSION_ROOT);
+    std::string user_save_path =
+        Common::GetTitleDataPath(Movie::GetTitleId(), Common::FROM_CONFIGURED_ROOT);
+    std::string user_backup_path =


Source/Core/Core/IOS/ES/ES.cpp

-    File::DeleteDirRecursively(save_path + "../backup/");
-#endif
-  }
+  Core::InitializeWiiSaves();


JMC47 · 2017-02-05T22:43:56Z

LGTM. Tested and verified that it loads the save, we can save in-game and load it later in another session and everything is working.

JMC47 · 2017-02-08T03:29:36Z

Is there anything blocking this for merge?

MrManslayerX · 2017-06-09T01:09:15Z

@JMC47 Hey, I just got this set up, and it's giving me a Frame 18 desync for Dakapon Kingdom. Any advice? I had the game working on 2 different Dolphin builds.
One was from this thread: https://www.reddit.com/r/gamegrumps/comments/28d7wj/kinda_tutorial_on_how_to_play_dokapon_with_your/
This one would desync mid play.
Another was https://dolphin-emu.org/download/dev/d3894a05949c938295ca1587cf9d777a50ca9c60/
This one worked flawlessly, but I couldn't save on netplay.
This one just doesn't even get past Frame 18.
I'm playing with one other friend, and I'm using a controller with port 1 and wiimote 1 assigned to me, she was using her keyboard with wiimote 2 set. It was working just fine on the other two builds. But, we kinda need it to stay synced, AND be able to play. Hah.
I've done several hours of looking around on the dolphin forums and manual troubleshooting, but nothing's coming to fruition here on this version.

JMC47 · 2017-06-09T01:20:15Z

We can narrow this down a bit. If you unplug the Wii Remotes and use GC controllers only, does the desync occur? If you uncheck use savefile, does the desync occur?

If you can find the results of these two things, we can narrow down what's tripping you up really easy. This isn't exactly the right place to discuss it, if you could ping me on freenode on #dolphin-emu, that's probably where I can give the best assistance.

JosJuice · 2017-06-09T06:12:12Z

Yeah, please don't discuss this on here. We use GitHub for development, not support.

RisingFog force-pushed the tas_wii_nand branch 3 times, most recently from 63bf6e6 to 6ccecda Compare December 24, 2016 02:47

leoetlino reviewed Dec 24, 2016

View reviewed changes

JosJuice reviewed Dec 24, 2016

View reviewed changes

RisingFog changed the title ~~[RFC/WIP] Copy Wii save for current game for Netplay and TAS~~ Copy Wii save for current game for Netplay and TAS Dec 28, 2016

JosJuice mentioned this pull request Jan 8, 2017

Don't call Movie::SetTitleId from ES #4629

Merged

JosJuice reviewed Jan 8, 2017

View reviewed changes

RisingFog force-pushed the tas_wii_nand branch from 2ce406a to cc595e3 Compare January 8, 2017 23:48

JosJuice requested changes Jan 10, 2017

View reviewed changes

RisingFog force-pushed the tas_wii_nand branch 2 times, most recently from ef97f71 to b22cbd1 Compare January 18, 2017 14:57

JosJuice previously requested changes Jan 18, 2017

View reviewed changes

RisingFog force-pushed the tas_wii_nand branch 4 times, most recently from 4b2ccde to 6454e88 Compare January 18, 2017 16:19

RisingFog force-pushed the tas_wii_nand branch 3 times, most recently from 5623b0b to ad8f8c8 Compare January 18, 2017 16:41

RisingFog force-pushed the tas_wii_nand branch 6 times, most recently from 06f153e to 501e35a Compare January 19, 2017 16:49

leoetlino reviewed Jan 20, 2017

View reviewed changes

RisingFog force-pushed the tas_wii_nand branch 3 times, most recently from dea1031 to dd449a8 Compare January 29, 2017 22:56

Copy Wii save for current game for Netplay and TAS

5224771

RisingFog force-pushed the tas_wii_nand branch from dd449a8 to 5224771 Compare February 5, 2017 18:17

Parlane merged commit f838d16 into dolphin-emu:master Feb 8, 2017

RisingFog deleted the tas_wii_nand branch February 8, 2017 18:59


		static std::string s_temp_wii_root;

		void InitializeWiiRoot(bool use_dummy)

Copy Wii save for current game for Netplay and TAS #4546

Copy Wii save for current game for Netplay and TAS #4546

Conversation

RisingFog commented Dec 24, 2016

JMC47 commented Dec 24, 2016

JMC47 commented Dec 24, 2016

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

JosJuice commented Dec 24, 2016

RisingFog commented Dec 24, 2016

JMC47 commented Jan 8, 2017 • edited

JosJuice commented Jan 8, 2017

JosJuice commented Jan 8, 2017

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

JMC47 commented Jan 10, 2017

JosJuice left a comment • edited

Choose a reason for hiding this comment

JosJuice commented Jan 14, 2017

E2xD commented Jan 18, 2017

JosJuice commented Jan 18, 2017

E2xD commented Jan 18, 2017

JMC47 commented Jan 18, 2017 via email

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

JosJuice commented Jan 18, 2017 • edited

JosJuice commented Jan 18, 2017 • edited

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

JMC47 commented Feb 5, 2017

JMC47 commented Feb 8, 2017

MrManslayerX commented Jun 9, 2017 • edited

JMC47 commented Jun 9, 2017

JosJuice commented Jun 9, 2017

JMC47 commented Jan 8, 2017 •

edited

JosJuice left a comment •

edited

JosJuice commented Jan 18, 2017 •

edited

JosJuice commented Jan 18, 2017 •

edited

MrManslayerX commented Jun 9, 2017 •

edited