Announcement: Be excellent to each other.


Caravel Forum : DROD Boards : Bugs : Bug when testing
New Topic New Poll Post Reply
Poster Message
Someone Else
Level: Smitemaster
Avatar
Rank Points: 2334
Registered: 06-14-2005
IP: Logged

File: The bug info.zip (148 KB)
Downloaded 47 times.
License: Public Domain
icon Bug when testing (0)  
I was testing a room, and I had been playing it in the normal play mode. I changed something in it (removed a ortho square, which caused a stalwart to get eaten. An adder, which would have got shrunk by the stalwart did not, and therefore when I continued playing, the adder moved too fast, and killed me. It currently shows where I would first die, and the adder is on top of Beethro. Beethro is not spinning, but the avatar looks like he does if Beethro has died. Also, I cannot make any moves. The screen turned entirely black except for the adder and Beethro (once I hit backspace), but when I switch back now (with alt-tab), the room is not black. The dying does not occur before hitting backspace.

This first hold is a hold I just mess around with the editor in.

How I caused it:
First copy the hold so you can edit it. From the menu, beat the first room and travel north. Then wait for the adder to get around and move diagonal up, staying as close to the head of the adder as possible. Go back to the editor and remove the NE-most Stalwart's ortho square. The bug should occur.

I have included my error log.

I cannot get this bug to reliably re-occur in other holds made to test the bug.

Edit:
The .err file is quite long, probably because I left the bug running for a while.

[Last edited by Someone Else at 09-30-2007 09:48 PM]
09-30-2007 at 09:47 PM
View Profile Send Private Message to User Send Email to User Show all user's posts High Scores This architect's holds Quote Reply
schep
Level: Smitemaster
Avatar
Rank Points: 865
Registered: 03-01-2005
IP: Logged
icon Re: Bug when testing (0)  
Hmm. This isn't so hard to reproduce in other situations. Modifying a room doesn't validate saved games the way importing a hold update does. So 'Continue Playing Game' messes up, and similar bad things can happen using 'Restore'.

I don't think we want to validate saved games every single time a hold modification is written to database. Loading saved games could do better at detecting an invalid save, but then what should DROD do about it? That one function I was recently messing with comes to mind: it plays as many commands as possible, and truncates the command list if and when things first seem corrupted.

Edit: It's actually already calling that function. But this results in truncating the command list from the first command which put the room is in an invalid state, and then keeping that invalid room state. Maybe it should undo to the last valid turn?

I also notice a commented line in CEditRoomScreen::SaveRoomToDB() that implies DROD did at one point validate saved games every time a room change was committed, but somebody decided that was taking too much time. So yeah, that's probably not the way to go.


[Last edited by schep at 09-30-2007 11:20 PM]
09-30-2007 at 10:46 PM
View Profile Send Private Message to User Send Email to User Show all user's posts This architect's holds Quote Reply
mrimer
Level: Legendary Smitemaster
Avatar
Rank Points: 5106
Registered: 02-04-2003
IP: Logged
icon Re: Bug when testing (0)  
schep wrote:
I also notice a commented line in CEditRoomScreen::SaveRoomToDB() that implies DROD did at one point validate saved games every time a room change was committed, but somebody decided that was taking too much time. So yeah, that's probably not the way to go.
Right. This functionality was disabled because people didn't want to have to wait X number of seconds each time a room is edited for all saved games in that room to be reverified. For some power players, it was quite noticeable. Like you suggest, we might change it when restoring game to undo to the previous turn, where the game is still in a valid state.

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.
10-01-2007 at 04:25 AM
View Profile Send Private Message to User Send Email to User Show all user's posts High Scores This architect's holds Quote Reply
schep
Level: Smitemaster
Avatar
Rank Points: 865
Registered: 03-01-2005
IP: Logged
icon Re: Bug when testing (+2)  
In CCurrentGame::LoadFromSavedGame(), just after the if (!PlayAllCommands(...)) {...} block, I added
		if (!this->bIsGameActive)
		{
			//Playing back the saved game didn't work as expected.
			//Undo to the last command when the game was active.
			UndoCommands(1, CueEvents);
		}
This seems to fix the buggy behaviors, so that "corrupt" saved games are loaded to the turn before death, both for "Continue Playing" and from the "Restore" screen.

I believe testing bIsGameActive is better than using the return value of PlayAllCommands, since if the very last command of a saved game involved leaving the room (e.g. a staircase or scripted exit was added in the editor), PlayAllCommands() would return true, but we don't want to allow leaving the room here.

Other related things to consider: When this code truncates commands, should the save game be modified and committed to database? (A "Continue" save gets replaced anyway, but with just the above change, checkpoint saves aren't permanently truncated, and modifying the room again could extend their apparent length.)
Do we still want that drod.err message, since this is something normal use can occasionally cause? Maybe move it to drod.log? A popup dialog?


[Last edited by schep at 10-01-2007 04:48 AM]
10-01-2007 at 04:45 AM
View Profile Send Private Message to User Send Email to User Show all user's posts This architect's holds Quote Reply
mrimer
Level: Legendary Smitemaster
Avatar
Rank Points: 5106
Registered: 02-04-2003
IP: Logged
icon Re: Bug when testing (0)  
schep wrote:
In CCurrentGame::LoadFromSavedGame(), just after the if (!PlayAllCommands(...)) {...} block, I added
		if (!this->bIsGameActive)
		{
			//Playing back the saved game didn't work as expected.
			//Undo to the last command when the game was active.
			UndoCommands(1, CueEvents);
		}
This seems to fix the buggy behaviors, so that "corrupt" saved games are loaded to the turn before death, both for "Continue Playing" and from the "Restore" screen.

I believe testing bIsGameActive is better than using the return value of PlayAllCommands, since if the very last command of a saved game involved leaving the room (e.g. a staircase or scripted exit was added in the editor), PlayAllCommands() would return true, but we don't want to allow leaving the room here.
That makes sense. I'll try it out too.
Other related things to consider: When this code truncates commands, should the save game be modified and committed to database? (A "Continue" save gets replaced anyway, but with just the above change, checkpoint saves aren't permanently truncated, and modifying the room again could extend their apparent length.)
I'm not sure about this. It could be due to a code or temporary hold bug that a move sequence is broken, so fixing the bug would obviate the need to commit the truncated sequence. I'm not sure what you mean about extending the apparent length of saves when a room is modified. Would you elaborate on that?
Do we still want that drod.err message, since this is something normal use can occasionally cause? Maybe move it to drod.log? A popup dialog?
Yeah, at least changing the message destination to drod.log is in order.

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.
10-01-2007 at 05:58 AM
View Profile Send Private Message to User Send Email to User Show all user's posts High Scores This architect's holds Quote Reply
schep
Level: Smitemaster
Avatar
Rank Points: 865
Registered: 03-01-2005
IP: Logged
icon Re: Bug when testing (0)  
mrimer wrote:
Other related things to consider: When this code truncates commands, should the save game be modified and committed to database? (A "Continue" save gets replaced anyway, but with just the above change, checkpoint saves aren't permanently truncated, and modifying the room again could extend their apparent length.)
I'm not sure about this. It could be due to a code or temporary hold bug that a move sequence is broken, so fixing the bug would obviate the need to commit the truncated sequence. I'm not sure what you mean about extending the apparent length of saves when a room is modified. Would you elaborate on that?
Yeah, I meant that if you go and revert the room change, the checkpoint's saved game can be used in its original length again. As you say, this would sometimes actually be a good thing. Just maybe a bit confusing in some cases, so I thought it was worth discussing.
10-01-2007 at 03:21 PM
View Profile Send Private Message to User Send Email to User Show all user's posts This architect's holds Quote Reply
mrimer
Level: Legendary Smitemaster
Avatar
Rank Points: 5106
Registered: 02-04-2003
IP: Logged
icon Re: Bug when testing (0)  
schep wrote:
mrimer wrote:
Other related things to consider: When this code truncates commands, should the save game be modified and committed to database? (A "Continue" save gets replaced anyway, but with just the above change, checkpoint saves aren't permanently truncated, and modifying the room again could extend their apparent length.)
I'm not sure about this. It could be due to a code or temporary hold bug that a move sequence is broken, so fixing the bug would obviate the need to commit the truncated sequence. I'm not sure what you mean about extending the apparent length of saves when a room is modified. Would you elaborate on that?
Yeah, I meant that if you go and revert the room change, the checkpoint's saved game can be used in its original length again. As you say, this would sometimes actually be a good thing. Just maybe a bit confusing in some cases, so I thought it was worth discussing.
Gotcha. No, let's not permanently lobotomize broken saved games. I've included this fix in build 55.

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.
11-01-2007 at 06:56 PM
View Profile Send Private Message to User Send Email to User Show all user's posts High Scores This architect's holds Quote Reply
New Topic New Poll Post Reply
Caravel Forum : DROD Boards : Bugs : Bug when testing
Surf To:


Forum Rules:
Can I post a new topic? No
Can I reply? No
Can I read? Yes
HTML Enabled? No
UBBC Enabled? Yes
Words Filter Enable? No

Contact Us | CaravelGames.com

Powered by: tForum tForumHacks Edition b0.98.8
Originally created by Toan Huynh (Copyright © 2000)
Enhanced by the tForumHacks team and the Caravel team.