feat(gta-streaming-five): allow to edit scene graph pools #3136

DaniGP17 · 2025-02-06T17:33:51Z

Goal of this PR

Allow editing of most SceneGraphNodes pools, the increase of fwPortalSceneGraphNode was suggested in the forum.
Recently it was attempted to add the pool fwPortalSceneGraphNode to the gameconfig(#2695) but as prikolium said they are allocated from another precomputed heap and should be looked for in fwSceneGraph::InitPools.
This was already done by blattersturm here but 2 days later they reverted his change due to Potential cause for 'missing doors after a while'. What they did was hook the PortalSceneGraphPool constructor to multiply the size and allocate the memory elsewhere.
I don't know if this is the approach that you want to take in order to increase the sizes of these pools.

How is this PR achieving the goal

First of all, I have defined a list of pools so that their sizes can be easily edited.

v0 = Allocate(214473LL, 16LL);
qword_7FF6B50437F8 = v0;
AssignStorage(v0, 210864LL);
v1 = (unsigned int)dword_7FF6B5043808 + v0;
v2 = v1 + 800;
v3 = v1;
v4 = v1 + 802;
v5 = v1 + 1802;
v6 = v1 + 1803;
v7 = v1 + 2803;
v8 = v7 + 406;

As you can see in the code, by default the game allocates 214473 in memory which is the sum of the pool storage sizes and the pool flag sizes and in AssignStorageMemory the pool flag sizes are sent. Both values are calculated with the data from the pools table and set in both arguments.
The memory displacement offsets are modified for each pool (v2, v4, v5, v6, etc.)
Later the value of all poolSizes is changed in each pool constructor.
And finally, other memory offsets of the AssignStorage function are changed.

This PR applies to the following area(s)

FiveM

Successfully tested on

Game builds: 1604, 2060, 2189, 3095, 3258, 3407(My tests have been to join the server and have a test hook to see the arguments that are sent to the pool constructor and they seemed correct.)

Platforms: Windows

Checklist

Code compiles and has been tested successfully.
Code explains itself well and/or is documented.
My commit message explains what the changes do and what they are for.
No extra compilation warnings are added by these changes.

Nobelium-cfx

Thank you for the effort. This looks like a great start! I have some comments, let me know WDYT.

It seems that there are more places where those sizes are used and therefore probably should be updated as well. E.g. in this function: 4C 8B C1 0F B6 49 ? 85 C9. But there is seems to be plenty. I didn't check it too deep, so idk if its feasible to track and update them all.
This is less of a priority that the 1st point and can be done later. I think it would make sense to move this code into PoolManagement.cpp and conditionally increase as we do with other pools based on fx::PoolSizeManager::GetIncreaseRequest().
Left some style nits related to this code.

code/components/gta-streaming-five/src/SceneGraphPools.cpp

DaniGP17 · 2025-02-09T21:48:28Z

Thank you very much for the review, about the way I'm changing all the values making many hook::put is it okay? It seems to me that I am using it a lot and I don't know if there would be another better way.

Nobelium-cfx · 2025-02-10T09:00:24Z

Thank you very much for the review, about the way I'm changing all the values making many hook::put is it okay? It seems to me that I am using it a lot and I don't know if there would be another better way.

I think hook::put ok. Idk any better way to do it and to me it look good as it is.

Change the initPoolsLocation at the top of the function. Index the pools by eSceneGraphPool instead raw index Change memory offsets in other functions(computeSceneNode, assignScratch, clearVisData)

DaniGP17 · 2025-02-17T15:34:07Z

It seems that there are more places where those sizes are used and therefore probably should be updated as well. E.g. in this function: 4C 8B C1 0F B6 49 ? 85 C9. But there is seems to be plenty. I didn't check it too deep, so idk if its feasible to track and update them all.

I've added new locations to change the pool sizes, but I'm stil looking for new locations cause as you said there're a lot of places.

This is less of a priority that the 1st point and can be done later. I think it would make sense to move this code into PoolManagement.cpp and conditionally increase as we do with other pools based on fx::PoolSizeManager::GetIncreaseRequest().

I'll check that after finding all the locations.

Left some style nits related to this code.

Done

iridium-cfx

We have also investigated this internally.
There is at least one other place that needs patching - the function at 83 B9 ? ? ? ? ? 4C 8B CA 4C 8B C1, which also then indexes into a fixed-size bitset based on the node index.

code/components/gta-streaming-five/src/SceneGraphPools.cpp

DaniGP17 · 2025-02-18T12:20:14Z

Sorry for the requested review, miss click.

We have also investigated this internally. There is at least one other place that needs patching - the function at 83 B9 ? ? ? ? ? 4C 8B CA 4C 8B C1, which also then indexes into a fixed-size bitset based on the node index.

Ok, I'll check that thanks.

Changed the first pattern to the begin of the function. Added the GetGbufScreenQuadPair function patch Changed the value of the FIRST_ROOM_SCENE_NODE_INDEX(FW_EXTERIOR_SCENE_GRAPH_NODE + FW_STREAMED_SCENE_GRAPH_NODE + FW_INTERIOR_SCENE_GRAPH_NODE - 1)

As Iridium said the at `83 B9 ? ? ? ? ? 4C 8B CA 4C 8B C1` function needed to be patched because the size of the skyPortalFlags was the size of the FW_PORTAL_SCENE_GRAPH_NODE pool. Since we doubled it(800) that size was not updated. I created a function that returns the address of the new skyPortalFlag with its correct size based on the address of the class self, so if there is another instance of the same class it will have a different skyPortalFlags. I patched the AddSkyPortal function to use the new SkyPortalFlag array and the same with the SetIsSkyPortalVisInterior. Fixed the offset of the pool size constructor of FW_FIXED_ENTITY_CONTAINER.

code/components/gta-streaming-five/src/SceneGraphPools.cpp

…etIsSkyPortalVisInterior patch

code/components/gta-streaming-five/src/SceneGraphPools.cpp

Changed r8 register to match the original(r8d) Changed the value that take r8d to `(pools[eSceneGraphPool::FW_PORTAL_SCENE_GRAPH_NODE].poolSize + 7) / 8` Add original missing code `mov(dword_ptr[rcx+0x378], r9d)`

iridium-cfx

I've added a couple others to review this as well.
It's quite a large change, so I'm not sure if it would be better to put it on a few branch for a while.

code/components/gta-streaming-five/src/SceneGraphPools.cpp

DaniGP17 · 2025-02-25T18:34:52Z

It's quite a large change, so I'm not sure if it would be better to put it on a few branch for a while.

Honestly, at first I had in mind only increasing the size of a pool (#3121), I saw that I could increase the others but then new locations to patch began to appear and now it is a big change, yes, I don't know if you have any suggestions on this subject

prikolium-cfx · 2025-03-04T11:33:42Z

It's quite a large change, so I'm not sure if it would be better to put it on a few branch for a while.

Honestly, at first I had in mind only increasing the size of a pool (#3121), I saw that I could increase the others but then new locations to patch began to appear and now it is a big change, yes, I don't know if you have any suggestions on this subject

Are you ok with idea to push this to experimental branch?

DaniGP17 · 2025-03-04T11:35:44Z

Are you ok with idea to push this to experimental branch?

Yes, it's fine for me

prikolium-cfx · 2025-03-04T11:49:30Z

Are you ok with idea to push this to experimental branch?

Yes, it's fine for me

It will be published to specific/feature/mlo-limits-increase as soon as build finish. I will also test it with big MLO packs that was crashing earlier.

DaniGP17 · 2025-03-04T11:52:54Z

It will be published to specific/feature/mlo-limits-increase as soon as build finish.

Nice, and if I want to make some changes I push a commit to my branch and that will automatically be published or I've to ask a cfx member to update that?

I will also test it with big MLO packs that was crashing earlier.

Related to that could you share the MLO packs later(when you have time) so I can test with that?

prikolium-cfx · 2025-03-04T11:53:44Z

It will be published to specific/feature/mlo-limits-increase as soon as build finish.

Nice, and if I want to make some changes I push a commit to my branch and that will automatically be published or I've to ask a cfx member to update that?

I will also test it with big MLO packs that was crashing earlier.

Related to that could you share the MLO packs later(when you have time) so I can test with that?

I just have Gabz subscription and use all of them together, so it crash me after 5-10 minutes teleporting between them

DaniGP17 · 2025-03-04T11:54:33Z

I just have Gabz subscription and use all of them together, so it crash me after 5-10 minutes teleporting between them

Oh ok, I have it too, I'll try it

prikolium-cfx · 2025-03-04T11:55:21Z

I just have Gabz subscription and use all of them together, so it crash me after 5-10 minutes teleporting between them

Oh ok, I have it too, I'll try it

And when you want to update your branch - poke me, I will update it on our end.

DaniGP17 · 2025-03-06T10:56:45Z

Related to this, I need to check it out because it seems that some recent change has caused it to crash most of the time in a tunnel inside a mountain.

Whisperisgod · 2025-03-06T18:00:53Z

I have a very high number of maps, if you want, I can help you with the map and give you vps for you to test.

DaniGP17 · 2025-03-06T18:02:35Z

I have a very high number of maps, if you want, I can help you with the map and give you vps for you to test.

Thanks, I appreciate the proposal but the VPS is not necessary. Are you getting a crash when having too many maps?

…rs values Previously I was storing the address of `skyPortalFlagArray` in rcx, which was not correct because later in `mov(dword_ptr[rcx+0x378], r9d);` rcx was used to set `m_skyPortalCount` to 0, since having the address of skyPortalFlagArray as rcx this was not working correctly. Now we are storing the value of the original code in rcx. I have removed `add(rcx, 0x37C);` because it is the offset to access the `skyPortalFlagArray` of the class but since we are allocating it in the c++ code, the only thing that offset was doing was jumping over a memory hole that was not used. Finally after setting `m_skyPortalCount` to 0 the value of skyPortalFlagArray is stored in rcx to later call `memset`.

DaniGP17 · 2025-03-06T23:34:16Z

Related to this, I need to check it out because it seems that some recent change has caused it to crash most of the time in a tunnel inside a mountain.

Already fixed this, now for me is not crashing, tested in 1604 & 3095.
@prikolium-cfx when you have time could you update the experimental branch to fix that crash?

prikolium-cfx · 2025-03-07T09:14:20Z

Related to this, I need to check it out because it seems that some recent change has caused it to crash most of the time in a tunnel inside a mountain.

Already fixed this, now for me is not crashing, tested in 1604 & 3095. @prikolium-cfx when you have time could you update the experimental branch to fix that crash?

Merged your latest changes into feature branch

Refactor the `AddSkyPortal` function. Sometimes `skyPortalCount` may take the value of -1, that produce a crash later when `GetStoredScreenQuadIndex` return a invalid index.

feat(gta-streaming-five): allow to edit scene graph pools

70ec09e

github-actions bot added the invalid Requires changes before it's considered valid and can be (re)triaged label Feb 6, 2025

Nobelium-cfx self-requested a review February 9, 2025 20:23

Nobelium-cfx suggested changes Feb 9, 2025

View reviewed changes

code/components/gta-streaming-five/src/SceneGraphPools.cpp Outdated Show resolved Hide resolved

code/components/gta-streaming-five/src/SceneGraphPools.cpp Outdated Show resolved Hide resolved

tweak(gta-streaming-five):

00ffaa2

Change the initPoolsLocation at the top of the function. Index the pools by eSceneGraphPool instead raw index Change memory offsets in other functions(computeSceneNode, assignScratch, clearVisData)

iridium-cfx reviewed Feb 18, 2025

View reviewed changes

code/components/gta-streaming-five/src/SceneGraphPools.cpp Outdated Show resolved Hide resolved

DaniGP17 requested a review from Nobelium-cfx February 18, 2025 12:17

DaniGP17 added 2 commits February 18, 2025 18:52

iridium-cfx reviewed Feb 21, 2025

View reviewed changes

code/components/gta-streaming-five/src/SceneGraphPools.cpp Outdated Show resolved Hide resolved

code/components/gta-streaming-five/src/SceneGraphPools.cpp Outdated Show resolved Hide resolved

code/components/gta-streaming-five/src/SceneGraphPools.cpp Outdated Show resolved Hide resolved

tweak(gta-streaming-five): rewrite AddSkyPortal function and finish S…

7a1ca1d

…etIsSkyPortalVisInterior patch

iridium-cfx reviewed Feb 24, 2025

View reviewed changes

code/components/gta-streaming-five/src/SceneGraphPools.cpp Outdated Show resolved Hide resolved

fix(gta-streaming-five): finish SetIsSkyPortalVisInterior patch

6a492ce

Changed r8 register to match the original(r8d) Changed the value that take r8d to `(pools[eSceneGraphPool::FW_PORTAL_SCENE_GRAPH_NODE].poolSize + 7) / 8` Add original missing code `mov(dword_ptr[rcx+0x378], r9d)`

iridium-cfx requested review from prikolium-cfx and dalekenium-cfx February 25, 2025 18:16

iridium-cfx reviewed Feb 25, 2025

View reviewed changes

code/components/gta-streaming-five/src/SceneGraphPools.cpp Outdated Show resolved Hide resolved

tweak(gta-streaming-five): rework AddSkyPortal & validate skyPortalCount

a5011bc

Refactor the `AddSkyPortal` function. Sometimes `skyPortalCount` may take the value of -1, that produce a crash later when `GetStoredScreenQuadIndex` return a invalid index.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(gta-streaming-five): allow to edit scene graph pools #3136

feat(gta-streaming-five): allow to edit scene graph pools #3136

DaniGP17 commented Feb 6, 2025

Nobelium-cfx left a comment

DaniGP17 commented Feb 9, 2025

Nobelium-cfx commented Feb 10, 2025

DaniGP17 commented Feb 17, 2025

iridium-cfx left a comment •

edited

Loading

DaniGP17 commented Feb 18, 2025

iridium-cfx left a comment

DaniGP17 commented Feb 25, 2025

prikolium-cfx commented Mar 4, 2025

DaniGP17 commented Mar 4, 2025

prikolium-cfx commented Mar 4, 2025

DaniGP17 commented Mar 4, 2025

prikolium-cfx commented Mar 4, 2025

DaniGP17 commented Mar 4, 2025

prikolium-cfx commented Mar 4, 2025

DaniGP17 commented Mar 6, 2025 •

edited

Loading

Whisperisgod commented Mar 6, 2025

DaniGP17 commented Mar 6, 2025

DaniGP17 commented Mar 6, 2025

prikolium-cfx commented Mar 7, 2025

feat(gta-streaming-five): allow to edit scene graph pools #3136

Are you sure you want to change the base?

feat(gta-streaming-five): allow to edit scene graph pools #3136

Conversation

DaniGP17 commented Feb 6, 2025

Goal of this PR

How is this PR achieving the goal

This PR applies to the following area(s)

Successfully tested on

Checklist

Nobelium-cfx left a comment

Choose a reason for hiding this comment

DaniGP17 commented Feb 9, 2025

Nobelium-cfx commented Feb 10, 2025

DaniGP17 commented Feb 17, 2025

iridium-cfx left a comment • edited Loading

Choose a reason for hiding this comment

DaniGP17 commented Feb 18, 2025

iridium-cfx left a comment

Choose a reason for hiding this comment

DaniGP17 commented Feb 25, 2025

prikolium-cfx commented Mar 4, 2025

DaniGP17 commented Mar 4, 2025

prikolium-cfx commented Mar 4, 2025

DaniGP17 commented Mar 4, 2025

prikolium-cfx commented Mar 4, 2025

DaniGP17 commented Mar 4, 2025

prikolium-cfx commented Mar 4, 2025

DaniGP17 commented Mar 6, 2025 • edited Loading

Whisperisgod commented Mar 6, 2025

DaniGP17 commented Mar 6, 2025

DaniGP17 commented Mar 6, 2025

prikolium-cfx commented Mar 7, 2025

iridium-cfx left a comment •

edited

Loading

DaniGP17 commented Mar 6, 2025 •

edited

Loading