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

Racing UI API #100

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Racing UI API #100

wants to merge 13 commits into from

Conversation

Greavesy1899
Copy link
Collaborator

First pass at exposing Racing HUDElement API using RaceXBin.

WIP PR, not going to commit as is. Feedback welcome

@Greavesy1899 Greavesy1899 added the enhancement New feature or request label Feb 27, 2024
@Greavesy1899 Greavesy1899 added this to the racing gamemode milestone Feb 27, 2024
@Greavesy1899 Greavesy1899 self-assigned this Feb 27, 2024
uint8_t padding_1[0x3]; // most likely other 'is visible' flags
uint8_t padding_2[0x34A];
float m_TargetTime = 0.0f;
uint16_t m_CurCheckpoint = 0; // maybe
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

going to figure this out before merge

code/client/src/core/ui/world_debug.cpp Outdated Show resolved Hide resolved
Comment on lines 252 to 253
using namespace SDK::mafia::ui;
using namespace SDK::mafia::database;
Copy link
Member

Choose a reason for hiding this comment

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

We should avoid this, it's always better to use the fully-fledged SDK namespace path on mod side


namespace SDK {
namespace mafia::database {
C_UIDatabase::C_HUDTable* C_UIDatabase::GetHUDTable() {
Copy link
Member

Choose a reason for hiding this comment

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

I feel like it can go inside header file and be a const method

Comment on lines 11 to 23
uint8_t padding_0[0x1E];
bool m_RacingVisible = false;
uint8_t padding_1[0x3]; // most likely other 'is visible' flags
uint8_t padding_2[0x34A];
float m_TargetTime = 0.0f;
uint16_t m_CurCheckpoint = 0; // maybe
uint16_t m_TotalCheckpoints = 0; // maybe
uint16_t m_CurPosition = 0;
uint16_t m_TotalPositions = 0;
uint32_t m_Unknown = 0;
uint16_t m_CurLap = 0;
uint16_t m_TotalLaps = 0;
uint8_t m_Countdown = 0;
Copy link
Member

Choose a reason for hiding this comment

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

Overall comment, please use the same member notation style as other SDK files, don't forget to add the offsets on each line and use the padX define style

Comment on lines 29 to 30
uint8_t padding[0x20];
C_HUDTable *m_HUDTable = nullptr;
Copy link
Member

Choose a reason for hiding this comment

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

Padding naming and member notation style

code/client/src/sdk/mafia/ui/hud/c_hud_controller.h Outdated Show resolved Hide resolved
code/client/src/sdk/mafia/ui/hud/race_xbin.cpp Outdated Show resolved Hide resolved
Comment on lines +18 to +19
// need to cast to C_UIDatabase
// TODO: Feels like this should be dynamic_cast, rather than reinterpret_cast
Copy link
Member

Choose a reason for hiding this comment

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

Can we handle that comment now? not a big deal

Copy link
Collaborator Author

@Greavesy1899 Greavesy1899 Mar 6, 2024

Choose a reason for hiding this comment

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

Causes a crash using dynamic_cast, unsure why. I'll debug though.

code/client/src/sdk/mafia/ui/hud/race_xbin.cpp Outdated Show resolved Hide resolved
Comment on lines 11 to 12
#include "sdk/ue/sys/sodb/c_database_interface.h"
#include "sdk/ue/c_weak_ptr.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

In every SDK files, we have to use relative imports

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Could you elaborate?

Copy link
Contributor

Choose a reason for hiding this comment

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

Comment on lines 399 to 401
// C_RaceTimer
uint64_t C_RaceTimer_SetVisible = 0x0;
uint64_t C_RaceTimer_StartRace = 0x0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you put this code in alphabetical order? (I mean between C_Quat and C_SceneObject)


// VISIBLE
ImGui::Text("Racing HUDElement Visibility");
ImGui::PushID("Racing_Visible");
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need ID here? (and in other places below)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

May have been for a previous revision. They can likely be removed.

@@ -0,0 +1 @@
#include "sdk/mafia/database/c_ui_database.h"
Copy link
Contributor

@Deewarz Deewarz Mar 6, 2024

Choose a reason for hiding this comment

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

I don't think I was clear for here:

If the import is in SDK (code/client/src/sdk/...), you must use relative imports (.../.../)
Elsewhere in the code you must use absolute imports (as you did well in word_debug.cpp)

Copy link
Contributor

@Deewarz Deewarz Mar 6, 2024

Choose a reason for hiding this comment

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

Here you can use #include "c_ui_database.h"

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 much prefer not use use relative imports, in my opinion it polutes the code base and generally causes imports to become unreadable. I'd rather use absolute paths starting with 'sdk'

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's fine in CPP files and the accompanying header file such as c_ui_database.h for c_ui_database.cpp though.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, but there was a discussion about it: #56 (comment)

@Segfaultd ready to reconsider your point of view on this?

Copy link
Collaborator Author

@Greavesy1899 Greavesy1899 Mar 6, 2024

Choose a reason for hiding this comment

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

If 'sdk' is its own contained module which is expected to transfer between projects, then absolute paths should be okay? As long as the includes remain inside the sdk folder.

Copy link
Member

Choose a reason for hiding this comment

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

We should revise using relative paths and switch to absolute ones; it's still portable as long as you set it to include dirs up in whatever project 'SDK' gets moved into. With relative paths, you introduce some extra friction whenever you need to refactor the source file elsewhere. In general, absolute paths give you a better idea of where the various included files are. In contrast, here it's a sea of ../../../ prefixes that force you to build a mental map of the layout instead.

cc @Segfaultd

@@ -0,0 +1,39 @@
#pragma once

#include "sdk/ue/sys/sodb/c_database_interface.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment on lines 10 to 12
#include "sdk/ue/c_variant.h"
#include "sdk/ue/c_weak_ptr.h"
#include "sdk/ue/sys/sodb/c_database_interface.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -1,9 +1,44 @@
#pragma once

#include "sdk/patterns.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -1,9 +1,44 @@
#pragma once

#include "sdk/patterns.h"

#include <utils/hooking/hooking.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

You probably don't need to re-import hooking here (already imported by pattern.h)

Comment on lines 3 to 4
#include "sdk/mafia/database/c_ui_database.h"
#include "sdk/mafia/ui/c_game_gui_2_module.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment on lines 447 to 449
// C_RaceTimer
gPatterns.C_RaceTimer_SetVisible = reinterpret_cast<uint64_t>(hook::get_pattern("48 89 5C 24 ? 48 89 6C 24 ? 48 89 74 24 ? 57 41 56 41 57 48 83 EC ? 0F B6 F2"));
gPatterns.C_RaceTimer_StartRace = reinterpret_cast<uint64_t>(hook::get_pattern("48 89 5C 24 ? 57 48 83 EC ? 48 8B F9 C7 41 ? ? ? ? ? 41 0F B7 C9"));
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you put this code in alphabetical order? (I mean between C_Quat and C_SceneObject)

@@ -90,10 +90,13 @@ namespace MafiaMP::Core {
ToggleVehicleDebug();
}

static bool bTempLockControls = false;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is all temporary so I can test because ImGui cursor is currently broken. I'll likely remove it out from this PR when it has been approved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

4 participants