Skip to content

Commit

Permalink
Reverted Worldmap.cpp changes in commit c19137b
Browse files Browse the repository at this point in the history
  • Loading branch information
NovaRain committed Nov 10, 2021
1 parent cd3c220 commit 32924db
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions sfall/Modules/Worldmap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ bool Worldmap::LoadData(HANDLE file) {
ReadFile(file, &mID, 4, &sizeRead, 0);
ReadFile(file, &elevData, sizeof(levelRest), &sizeRead, 0);
if (sizeRead != sizeof(levelRest)) return true;
mapRestInfo.emplace(mID, elevData);
mapRestInfo.insert(std::make_pair(mID, elevData));
}
if (count && !restMap) {
HookCall(0x42E57A, critter_can_obj_dude_rest_hook);
Expand Down Expand Up @@ -637,7 +637,7 @@ void Worldmap::SetRestMapLevel(int mapId, long elev, bool canRest) {
elev++;
}
elevData.level[elev] = canRest;
mapRestInfo.emplace(mapId, elevData);
mapRestInfo.insert(std::make_pair(mapId, elevData));
}
}

Expand Down

7 comments on commit 32924db

@NovaRain
Copy link
Collaborator Author

@NovaRain NovaRain commented on 32924db Nov 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sfalldb.sav was added in 4.1.1 for get/set_can_rest_on_map functions, at first it has a minimum size of 4 bytes.
For some reason, the code generated with IA32 using .emplace method in Worldmap::LoadData() will cause a C++ runtime error when loading an old sfalldb.sav that is less than 8 bytes. Of course if one deletes the file from the save, things would work normally.
Changing the architecture for code generation to SSE (/arch:SSE) can also "fix" the error without reverting the code, so yeah, yet another quirk/glitch from VS's code generation. If problems like this occur more times, I'd consider switching to SSE (PIII/Athlon XP) as the minimum requirement, and maybe add a function checking CPU features at the beginning of sfall init so ppl (if there's any) don't get crashes at start with no reason.

@FakelsHub
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sfalldb.sav - can I get this file?

@NovaRain
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sfalldb.sav - can I get this file?

I guess you didn't check my RP save?
It just has 4 bytes of 0x00, came from the saves with older sfall 4.1.x, or you can simply create one for test.
From my tests your build didn't have the runtime error with it, it's probably just VS2015 toolset was acting buggy when using /arch:IA32.

@FakelsHub
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the problem is in emplace. Probably a bug is buried somewhere.

@NovaRain
Copy link
Collaborator Author

@NovaRain NovaRain commented on 32924db Nov 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the problem is in emplace. Probably a bug is buried somewhere.

Maybe, in my previous tests I tried to comment out the whole for loop in Worldmap::LoadData() (obviously .emplace is gone), but the build still has runtime error with "old" sfalldb.sav.
But it's the only change made on Worldmap.cpp after comparing 4.3.0.2 and 4.3.1 source code, and it does make the runtime error go away, so I revert the code.

@FakelsHub
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is not what you thought, it's just that you were unlucky and the garbage value got to the wrong place)

@NovaRain
Copy link
Collaborator Author

@NovaRain NovaRain commented on 32924db Nov 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is not what you thought, it's just that you were unlucky and the garbage value got to the wrong place)

What do you mean? Does the original code in Worldmap::LoadData() has some kind of flaw?
EDIT: Ah, I saw the code change in CritterStats.cpp.

Please sign in to comment.