Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promote v0 to v1, extend v1 and extend GpuApi #178

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

WSSDude
Copy link
Collaborator

@WSSDude WSSDude commented Feb 1, 2025

1st commit:

  • promoted v0 to v1
  • fixed incorrect naming of State::OnUpdate to State::OnTick (copies game)
  • renamed GameStates::Add to AddHook and modified comments
  • split GameState hooks to before/after hooks for more granular control
  • added EMainReason::Run for notifying plugins about entry to CRunningState

2nd commit:

  • added new system for hooking renderer events to v1
  • cleanup API a bit more and bring it closer to 2025

3rd commit: (TBD? considering making this another commit at this point, this is kinda huge...)
* added helper function to GpuApi for waiting till all frames were presented
* added some basic textures struct to GpuApi
* swapchain structure fixes based on some newer knowledge
^ split this into new PR

@WSSDude WSSDude force-pushed the features/api-update branch 2 times, most recently from b47f48b to caa8d5d Compare February 1, 2025 12:41
* promoted v0 to v1
* fixed incorrect naming of State::OnUpdate to State::OnTick (copies game)
* renamed GameStates::Add to AddHook and modified comments
* split GameState hooks to before/after hooks for more granular control
* added EMainReason::Run for notifying plugins about entry to CRunningState
@WSSDude WSSDude force-pushed the features/api-update branch from caa8d5d to ae4f1c3 Compare February 1, 2025 13:26
@WSSDude WSSDude changed the title Promote v0 to v1 Promote v0 to v1, extend v1 and extend GpuApi Feb 1, 2025
@WSSDude WSSDude force-pushed the features/api-update branch from caa50b4 to b4f0e57 Compare February 2, 2025 09:34
@WSSDude WSSDude marked this pull request as ready for review February 2, 2025 09:56
@WSSDude WSSDude marked this pull request as draft February 2, 2025 09:56
@WSSDude
Copy link
Collaborator Author

WSSDude commented Feb 2, 2025

Practically same as wopss/RED4ext#94 (comment), would like to have a more proper look over the Present again if something might not be useful to expose, even if it may not be intresting now due to missing structs (already pre-exposed index into monitors structure that is used with HDR and contains curves for various supported modes by the monitor in the ResizeBuffers...)

But those are for the next PR, this was supposed to be about API primarily...

Copy link
Owner

@wopss wopss left a comment

Choose a reason for hiding this comment

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

Thanks for the PR.

I saw some changes that include STL structs, which I would not prefer in the API, since it might be used in other code than C++ or compiled with a different C++ version.

include/RED4ext/Api/PluginHandle.hpp Outdated Show resolved Hide resolved
include/RED4ext/Api/v1/GameState.hpp Outdated Show resolved Hide resolved
include/RED4ext/Api/v1/PluginInfo.hpp Outdated Show resolved Hide resolved
include/RED4ext/Api/v1/Render.hpp Show resolved Hide resolved
include/RED4ext/Api/v1/Scripts.hpp Outdated Show resolved Hide resolved
include/RED4ext/Api/v1/Sdk.hpp Outdated Show resolved Hide resolved
@@ -10,6 +10,7 @@ namespace RED4ext
enum class EMainReason : uint8_t
{
Load = 0,
Unload
Unload,
Run
Copy link
Owner

Choose a reason for hiding this comment

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

What would be the advantage of having this instead of just using GameStates?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@psiberx wanted to include this for those that do not need to be hooking but only know that the game initialized and is running. He even suggested to call it from Running::OnEnter.

Copy link
Owner

Choose a reason for hiding this comment

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

What about calling it on Running::OnTick and change it to Tick?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I guess at that point, you may hook the game state. Believe psi wanted this as a one-off notification before first OnTick. Also am uncertain from this if you mean there would be some continuous notification, then I suppose it may as well truly be a game state hook 😅

Copy link
Collaborator

Choose a reason for hiding this comment

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

The only advantage is easier entry for beginners. I didn't suggest adding it though, just gave an example of how it could be handled. In Load you can't even do anything engine/game related. The game states are hard to understand and at least need a better description, explaining which systems exactly are initialized at what point.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Was unsure now if I should keep it or not, kept it for now.

Copy link
Owner

Choose a reason for hiding this comment

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

I'm fine with keeping it.

* revert STL changes, postopned for v2
* change plugin handle to void*
* return enum instead of bool from GameState hooks, change docs accordingly
@WSSDude WSSDude force-pushed the features/api-update branch from 0f751c5 to 9099004 Compare February 2, 2025 20:24
@WSSDude WSSDude force-pushed the features/api-update branch from 9099004 to d66f101 Compare February 2, 2025 23:01
@WSSDude
Copy link
Collaborator Author

WSSDude commented Feb 5, 2025

Just some update on why nothing gets posted for a bit - I found various issues trying to use this, trying to find some better hooks for initialize/deinitialize or some combination that should be reliable enough. Initialize only does base initialization of devices, not swapchain and some other systems that I had plan to hook. Same for shutdown - is just absolute last step of shutdown as I found out and not shutdown for all systems as I thought at first.

Found something already and got a bit sidetracked by it, should hopefully post some updates soon...

Also considering swapping the ResizeBackbuffer for some more upper-level function as it may get called twice a frame for no good reason as I found out, leading to some suboptimal things on hooked side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants