0.10.7 - Saving while diving Underwater in Dungeon then Load back Resets Breath Meter to Full

Curated user bug reports with enough information to reproduce issue. Moderators will move topics here from Help & Support once a bug is confirmed and enough information has been provided.
Post Reply
User avatar
Slasonic
Posts: 12
Joined: Mon Sep 02, 2019 2:37 am

0.10.7 - Saving while diving Underwater in Dungeon then Load back Resets Breath Meter to Full

Post by Slasonic » Mon Sep 09, 2019 9:10 am

Well, I saved my game while still diving underwater in Dungeon (Breath Meter in Half). After I load back, the Breath Meter Resets to Full.
Image
"Imagination is power, yet useless if not done properly."
- Some Random Word, [Insert Date Here]

User avatar
JorisVanEijden
Posts: 50
Joined: Mon Aug 12, 2019 5:02 pm

Re: 0.10.7 - Saving while diving Underwater in Dungeon then Load back Resets Breath Meter to Full

Post by JorisVanEijden » Mon Sep 09, 2019 6:22 pm

I can confirm the bug.

Initial investigation shows that when the dungeon is loaded the player is teleported to the start of it.
Then the check for being underwater is done. You're not, so you get your breath back.
And then you're restored to your saved position, which is underwater, so you get a splash and possibly an encumbrance message.

This looks a bit tricky, with different things running in different orders. No time for me to look into it further today so I'm just dumping the code flow in case someone else wants to look into it.

Code: Select all

Serialization.SaveLoadManager.LoadGame()
  PlayerEnterExit.RestorePositionHelper()
    PlayerEnterExit.RespawnPlayer()
      StartCoroutine => 
        PlayerEnterExit.Respawner()
          PlayerEnterExit.StartDungeonInterior()
            PlayerEnterExit.MovePlayerToMarker()
              transform.position = marker.transform.position + Vector3.up * (controller.height * 0.6f);
PlayerEnterExit.Update()
  isPlayerSwimming = false;
Serialization.SaveLoadManager.RestoreSaveData()
  Serialization.SerializableStateManager.RestorePlayerData()
    Serialization.SerializablePlayer.RestoreSaveData()
      Serialization.SerializablePlayer.RestorePosition()
        transform.position = positionData.position;
PlayerEnterExit.Update()
  isPlayerSwimming = true;

User avatar
Interkarma
Posts: 4838
Joined: Sun Mar 22, 2015 1:51 am

Re: 0.10.7 - Saving while diving Underwater in Dungeon then Load back Resets Breath Meter to Full

Post by Interkarma » Mon Sep 09, 2019 9:51 pm

I'm happy to take care of this one. I'll move to bug reports for tracking.

User avatar
JorisVanEijden
Posts: 50
Joined: Mon Aug 12, 2019 5:02 pm

Re: 0.10.7 - Saving while diving Underwater in Dungeon then Load back Resets Breath Meter to Full

Post by JorisVanEijden » Tue Sep 10, 2019 4:36 am

I guess it could be solved by making the RestorePositionHelper() method ensure it places the player at their save location rather than the start of the dungeon.

But there's also the behaviour where LoadGame() first sets up everything in its initial state, then waits at least a frame before applying the savedata.
I guess this allows the world to "settle in" so everything is in the proper initial state before applying saved data to it.

But some stuff happening in Update() and FixedUpdate() has effects that should not really be applied until after the savedata is applied. Like restoring the player's breath while they are temporarily at the start of the dungeon.

Even if we move the player to the right spot right before the frame wait rather than after, can we really be sure that nothing happens in those methods that should really wait until the saved data is restored?

Should those effects all be stored in the save (like isSwimming and isSubmerged) so that the effects of the update methods get undone again?

Should the Updates know if the game is loading and adjust their behaviour to that?

User avatar
JorisVanEijden
Posts: 50
Joined: Mon Aug 12, 2019 5:02 pm

Re: 0.10.7 - Saving while diving Underwater in Dungeon then Load back Resets Breath Meter to Full

Post by JorisVanEijden » Tue Sep 10, 2019 6:04 pm

I created a PR with a fix for this: https://github.com/Interkarma/daggerfal ... /pull/1506
But I'm not sure it is the right approach and if it causes side effects I have not spotted.

On a related note, there seems to be some duplication of functionality between
- GameManager.StateManager
- GameManager.IsGamePaused
- GameManager.IsPlayingGame()
- Serialization.SaveLoadManager.LoadInProgress
- 16 different ReadyCheck() methods
- 23 different IsReady fields, properties and methods

User avatar
Interkarma
Posts: 4838
Joined: Sun Mar 22, 2015 1:51 am

Re: 0.10.7 - Saving while diving Underwater in Dungeon then Load back Resets Breath Meter to Full

Post by Interkarma » Tue Sep 10, 2019 9:07 pm

Thanks, I'll take a look at the PR when I can.

Regarding ReadyCheck() and IsReady duplication - please keep in mind all these early classes began life as DFTFU (Daggerfall Tools for Unity) and were generally used in isolation. There was no overarching game state to call upon and each class had to fend for itself and do its own checks. Prior to that, much of the code existed as DaggerfallConnect, a C# library for my other exploring tools. And prior to that, it was part of my C++ exploring tools and a small game engine I was working on. Some of this code dates back to 2000-2001.

I literally had no intention of remaking Daggerfall from 2009 after rebooting DFWorkshop through first half of 2015. I was just making tools for grabbing assets out of the files and using them wherever. Things became more cohesive later in development as the vision solidified. When you're looking at the code, what you're seeing is almost 20 years of development across several tools and platforms, all with various goals.

I didn't set out to do any of this, it just worked out that way. If things seem unplanned in the old stuff, that's because it was unplanned. I was working on my own for almost 15 years just building stuff to see what would stick. Everything grew very gradually out of something else until it hit critical mass in the last few years and here we are.

Something that's often hard to appreciate at this end of the line, is that early on I had to reverse engineer unknown game data. I wasn't building to spec with data formats casually in hand - I was discovering to build and building to discover, creating my own ladder one rung at a time. That's an awkward messy process, writing code for systems you can only see through the wrong end of a telescope. It became easier later as help finally arrived.

Anyway, some of that old bootstrapping could be cleaned up a bit now that a real game has been built around it, but I'd like to try and massage the old DFTFU classes into keeping their tool roots and not rely on DFU core if possible. And I have no intention of chasing constant reviews and refactors of ancient code for the remainder of my natural life. If you or someone else wants to clean up the old bones for the sake of it, please fork the project to do so. At this stage, I only want small incremental changes in DFU up to 1.0 as I gradually move my attention fully into another game project.

Other developers get to move on and do new things. After 20 years on various Daggerfall stuff, I think I've earned the right to do something else. For everyone just discovering DFU now, they probably don't realise just how long and winding this whole journey has been, or how exhausted I am after all these years.

NoobioDF
Posts: 52
Joined: Sat Jun 22, 2019 4:03 am

Re: 0.10.7 - Saving while diving Underwater in Dungeon then Load back Resets Breath Meter to Full

Post by NoobioDF » Wed Sep 18, 2019 5:10 pm

@ Interkarma

That's a fascinating history of your endeavour, Interkarma, and one that makes the entire journey even more incredible an accomplishment. Very glad you went into that explanation, it's like a fireside chat with a developer years after his exploits, but earlier.

I agree with you that forking off the development to clean-up the DFU code makes a lot of sense. Whenever such a massive revision is done, it can lead to mistakes, as it would take reading all the code to see what uses old vestiges of things like DF Tools code (as you mentioned) versus cleaner direct DFU code. It's a worthy goal to do a complete clean-up, as it would avoid problems like the Underwater Breath issue popping up (or other early initialization state code) plus perhaps result in some optimization or evasion of problems down the road, especially with massive mods or complete conversions if any are made. But, it could produce some bugs, especially with the older DF Tools, or similar code.

Like your new Avatar to the right on the posts, looks muscular and successful, which given how far this project has come, is especially appropriate.

Having followed for a long time the attempts to modernize Jagged Alliance 2 with JA2v113 and its ups and downs, the idea of doing only some basic fixes on the current DFU code, as you mentioned above, but Fork the project if some want to do a complete revision and clean-up of old vestige code (which would prevent other issues and aid in optimization possibly and large mod compatibility) is very wise, given how JA2 ran into so many issues early on and then moved forward a lot more later with a different approach.

In the interim, I'm now almost aghast at what you accomplished. So at first you were sort of white-hacking the game to look at, sometimes in Unity, and then you formed Tools to do so, and then at some point when it all hit critical mass, you realized and decided to create a fully realized system to convert to Unity -- from Hacker to Developer -- that's an incredible journey and enlargement of scope. Only possible because of your passion, curiousity, and love for the game and for exploration. Incredible story, incredible result.

User avatar
Interkarma
Posts: 4838
Joined: Sun Mar 22, 2015 1:51 am

Re: 0.10.7 - Saving while diving Underwater in Dungeon then Load back Resets Breath Meter to Full

Post by Interkarma » Wed Sep 18, 2019 9:46 pm

Hey NoobioDF, happy you enjoyed reading that. :) I was mainly trying to ground the project and help contextualise why some things were done they way they were, but starting to realise the process can be an interesting story all to itself. Although in hindsight, I feel my reply above came across a bit defensive, which wasn't intended.

BTW, my avatar was created by Geremy Walker of Bastard Bonds fame and stylised into his strong aesthetic. He's one of those crazy-talented devs like Lucas Pope who can do basically everything by themselves - art, programming, shaders, animation, the works. I know I'll never be equal to devs like that, but they do inspire me to keep improving.

NoobioDF
Posts: 52
Joined: Sat Jun 22, 2019 4:03 am

Re: 0.10.7 - Saving while diving Underwater in Dungeon then Load back Resets Breath Meter to Full

Post by NoobioDF » Wed Sep 18, 2019 11:32 pm

@ Interkarma

Your post above my initial one did not appear defensive at all. If anything, it appeared more as attempting to excuse something by putting it in an historical context, leading to a very interesting, and rather inspiring story.

Thanks for the info on the artist, Jeremy Walker, as well. He is clearly very, very good at what he does.

I put the Bastard Bonds game on my wishlist to examine later on Steam. Looks interesting from my 30 second skim, thanks.

Post Reply