[GEN][ZH] Fix rebalanceChildSleepyUpdate debug runtime exceptions#563
[GEN][ZH] Fix rebalanceChildSleepyUpdate debug runtime exceptions#563Mauller wants to merge 1 commit intoTheSuperHackers:mainfrom
Conversation
1de0acb to
a1b8b1e
Compare
|
I believe i have sussed out the main issues in zero hour causing problems, there are others i have not pushed but they are asserts being hit due to issues with the main menus shell map. Will sort out generals tomorrow. |
a1b8b1e to
3780304
Compare
GeneralsMD/Code/GameEngineDevice/Include/MilesAudioDevice/MilesAudioManager.h
Outdated
Show resolved
Hide resolved
GeneralsMD/Code/GameEngine/Source/Common/System/SubsystemInterface.cpp
Outdated
Show resolved
Hide resolved
GeneralsMD/Code/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
I think the way this was meant to be written is
UpdateModulePtr* pSZ = &m_sleepyUpdates + m_sleepyUpdates.size();Then all other changes here can be reverted.
There was a problem hiding this comment.
It doesn't work, i already tried these.
The issue at this point is with the value of child going out of range of the container on line 2841.
I did it this way so i could use a local variable of container size to catch when child exceeded the size of the container, to minimise any performance penalty. ( What i took from the comments is that this gets hit a lot. )
In normal operation it runs fine, it's mostly debug that the issues occur, but we probably shouldn't be letting arrays return things out of bounds.
There was a problem hiding this comment.
Have you seen this child being larger than the size? If not, then this check should be an assert or removed.
There was a problem hiding this comment.
Yes multiple times and it is why i put this guard here. m_sleepyUpdates is a std::list, which throws asserts under debug when you pass it an index outside of the arrays bounds.
child = ((i+1)<<1)-1;
// TheSuperHackers @fix Mauller 02/04/2025 Prevent invalid index into container throwing errors during debug
if(child >= containerSize) break;
pChild = &m_sleepyUpdates[child];i can become greater than half the size of the container, which results in child becoming greater than the size of the container at this point.
This then triggers assert in the std library.
Under normal conditions since pChild becomes greater than pSZ, it doesn't casue a problem during normal gameplay.
while (pChild <= pSZ)
The original loop bellow on lines 2878, uses the iterator Child value and the container size as the test in the while loop, so this loop doesn't casue issues like the pointer based "unrolled loop" since everything gets checked for the child size against the container size before the container is accessed.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/RailroadGuideAIUpdate.cpp
Outdated
Show resolved
Hide resolved
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/RailroadGuideAIUpdate.cpp
Outdated
Show resolved
Hide resolved
f78229a to
c28a408
Compare
721ea34 to
aa3b2b8
Compare
|
This should be up to date now with changes and Generals inclusions. |
aa3b2b8 to
b444274
Compare
|
Rebased with main 05/04/2025 |
b444274 to
37aa1e5
Compare
|
Rebased with main after recent cleanups and null manager. |
37aa1e5 to
1ac75af
Compare
1ac75af to
c474fbb
Compare
|
Updated with critical fixes to the script engine. The change from STL port to STD library meant that iterators were being used in a way that was invalid in STD library. The changed behaviour compares a pointer to the data within the previously seen iterator, which is then compared to the current iterators data pointer. This change should match the behaviour that would have been seen when using STL port type iterators. |
d005531 to
877083a
Compare
|
I think this is at a point now where it is pretty stable after adding further fixes for the miles audio and also fixing the script engine. I have rebased it with latest Main, BFME shader additions, so it should be an easy merge. The Miles audio issue turned out to be that it was trying to erase an iterator on the wrong container. So i have reverted some of the just recent changes. For the script engine, i replicated what the original STL port iterators would have been doing by copying the address of the script the previous iterator was pointing to, then comparing that address with the address of the script the current iterator points to. In theory this should replicate what the original iterators were doing and preserve the original behaviour of the script engine. This build has been tested with the golden sample replay on a VC6 build and it does not mismatch and returns the expected score at the end of the game. |
|
The supply center dock update also appears to have an issue related to it, there was also incorrectly cast pointers. |
877083a to
c915d0a
Compare
GeneralsMD/Code/GameEngine/Source/Common/RTS/ResourceGatheringManager.cpp
Outdated
Show resolved
Hide resolved
c915d0a to
279db18
Compare
279db18 to
0c3d4b2
Compare
0c3d4b2 to
80ebb4b
Compare
|
Updated this PR to just contain the GameLogic sleepy updates commit. All other fixes have been pulled out into seperate PR's of related issues. |
|
How can this issue be reproduced? |
Pull #632 then run under debug, it will fall into the exception in sleepy updates pretty quick. |
|
Yes. Ran into this swiftly in shell map. I have put proper fix in new pull request #644 which makes it work the way it was intended. Your fix is correct in principle, but it adds 2 new branches, 1 in the loop, which is not necessary, and therefore is not the preferred fix for this. |
Yeah that's all fair, was just looking over your change and it looks good to me, using pointer arithmetic like the rest of the loop instead. The while then catches any issues if pChld goes out of bounds when entering and ending a loop iteration. |
Previously known as: [GEN][ZH] Fix debug critical runtime errors
Fix critical runtime errors in generals and zero hour that are preventing debug from working.Edit: I have split out a lot of the fixes that were within this PR into smaller PR's to make them easier to review individualy.
This PR now only contains fixes to the sleepy updates loop within GameLogic.
The issue with the sleep updates loop is that the passed in value i can create a child index that is beyond the end of the container it is used with. This will cause unexpected behaviour but is caught by the check on the while loop, the child pointer must be smaller than the end of container pointer.
This behaviour also occurs within the loop when the next i value and child index are calculated, so must be guarded within the loop as well.
The out of bounds access exception is only thrown during debug and prevents the use of debug. under normal gameplay the pointer returned should never be acted upon if it is beyond the bounds of the container.
This fix only concerns Debug and doesn't affect normal runtime, but it does prevent us from running debug at all.
Current known issues:- Depends on: #464* #489~~* #536 ~~
* #537* #538