Request: Remove warnings and add more customization please #246

New issue

Open

opened 2024-10-05 19:14:29 +02:00 by Zvendson · 1 comment

Zvendson commented

2024-10-05 19:14:29 +02:00

(Migrated from github.com)

So i have been into this repo for a few days now and its really amazing and well written.
Thanks for that!

I made a Fork where I applied all of my requests already but the implementation is really "ugly" and a more rather quick solution to my needs now.
Keep in mind I am fairly new to this Project, so I was a little bit careful in changing existing stuff to not break things.
Just in the middle of writing the Issue, I saw that you already have seen my fork and left a comment there.

Customizations requests:

Please could you make settings to make the CppSDK more customizable?
I e.g. dont want to have all the .cpp files in the same directory than the .hpp files.
I added a rather ugly way to make an include and source folder but it fits my
project where I plan to use the dumped SDK and i can simply copy&paste the CppSDK
folder (which i renamed too).
Also neat would be to rename the SDK.hpp to a custom name too.
I added a way to take the current path of the .dll to generate the dump in the same
directory, instead of using C:/.... It would be neat if that could be customizable.
I dont want to create a PR for that, because you see my changes? I wouldnt want to
have them too haha.
More infos on the settings itself. Me as someone who stumbled over this has not much
experience what certain settings will change on the output and why that would be useful.
This is more like a question or 50% question. Is the dumped output usable for linux machines?
To clarify. I have no exp with linux c++ but I want to change that sooner or later during my API
development for the PalServer, to support native Linux servers without having them to install
wine or proton. If not usable for linux, could there be a future change?
Would it be possible to add a kind of "filter" into the dumper? What I am talking about is that
the PalServer, contains classes that are not used at all, bcs they are obviously for the gameclient.
classes and functions that are e.g. for the UserInterface. I would like to append a list of strings
that will prevent those classes to be generated at all. If thats not too hard to implement ofc.
(Still getting overwhelmed to try to dig deeper into the dumper but I want to focus more on
project that will depend on the dumper. So I am asking you here.)

Warnings:
Disclaimer: I have been able to fix all warnings on /W3 for now but most changes are more ugly to achieve it.

While fixing all the errors I stumbled about a "problem" that repeats itself almost everywhere: type conversations.
I have several Ideas on how to change that overall:
- Avoid using smaller type sizes for integers and use i64 "only".
  Probably not the desired way but some code pieces def. should have a bigger size.
  Common cases I saw where for int i loops. the index var is an int but the condition is compared to an unsigned value.
  Example and here the Fixed Version
- place the static_casts at the correct positions where you are sure that the is no dataloss possible. Example
- Dealing correctly with wstring-string conversation:. Not saying my
  implementation is the best but i would claim it decent enough.

Fix generated enum classes. While using the dumper, i stumbled across a lot of warnings that exceed the maximum
size the underlaying enum type can actually hold, which results into a massive spam of overflow warnings.
My solution is really bad for this and is a more or less frequent solution. I added enough comments for you to follow
my thoughts
I add an example enum to emphasize more what I am talking about:

// Enum Engine.EStreamingSourcePriority
// NumValues: 0x0007
enum class EStreamingSourcePriority : uint8
{
    Highest                                  = 0,
    High                                     = 64,
    Normal                                   = 128,
    Low                                      = 192,
    Lowest                                   = 255,
    Default                                  = 128,
    EStreamingSourcePriority_MAX             = 256, // Warning bcs 255 is max size
};

my current solution writes the _MAX overflow beneath it as an inline constexpr. But i am not sure if the _MAX fields
is actually used by the engine or required to be inside the enum. another thought was changing the enum class type
to a bigger size but that could cause to misalignments?

Fixing the FVector. Im not sure if the "float" is required in every FVector implementation. I changed it to keep the
using UnderlayingType = double support.
I tested the code by compiling with using UnderlayingType = float and it did not cause a warning.

class / struct mismatch. This one was the hardest to deal with. Basically the problem is:
the dumper defines a class and later references this class as struct type. Those mostly occured with:
FProperty, FField and FStructProperty when they are used in fields like:
TFieldPath<struct FProperty>
Here and example:

struct FPropertyAccessSegment final
{
public:
	class FName                                   Name;                                              // 0x0000(0x0008)(ZeroConstructor, IsPlainOldData, NoDestructor, HasGetValueTypeHash, NativeAccessSpecifierPrivate)
	class UStruct*                                Struct;                                            // 0x0008(0x0008)(ZeroConstructor, NoDestructor, UObjectWrapper, HasGetValueTypeHash, NativeAccessSpecifierPrivate)
	TFieldPath<struct FProperty>                  Property;                                          // 0x0010(0x0020)(HasGetValueTypeHash, NativeAccessSpecifierPrivate)
	class UFunction*                              Function;                                          // 0x0030(0x0008)(ZeroConstructor, NoDestructor, UObjectWrapper, HasGetValueTypeHash, NativeAccessSpecifierPrivate)
	int32                                         ArrayIndex;                                        // 0x0038(0x0004)(ZeroConstructor, IsPlainOldData, NoDestructor, HasGetValueTypeHash, NativeAccessSpecifierPrivate)
	uint16                                        Flags;                                             // 0x003C(0x0002)(ZeroConstructor, IsPlainOldData, NoDestructor, HasGetValueTypeHash, NativeAccessSpecifierPrivate)
	uint8                                         Pad_3E[0x2];                                       // 0x003E(0x0002)(Fixing Struct Size After Last Property [ Dumper-7 ])
};

My fix for that is extremely shit but I gave up to find out how i can check if the Member-param in

CppGenerator::GetMemberTypeStringWithoutConst(UEProperty Member, int32 PackageIndex)

is a class type. I could not figure that one out. so i was hoping you have a better solution for that.
my "Fix"

Last Words:
I hope I did not forget anything here to mention. The most important things for me are that all the warnings are removed properly.
Sorry for my OCD but i must obey them xD
The Dumper is really amazing and does the Job really well. I really appreciate all the work that has been done here.
I made the Fork to fit my needs but also as an Example where I could tackle the problems and give you a faster
insight. I also made it a solution to fill this Issue with examples and provide a detailed report/request.

I hope you have a great day/weekend!

So i have been into this repo for a few days now and its really amazing and well written. Thanks for that! I made a [Fork](https://github.com/Zvendson/PalServer-Dumper) where I applied all of my requests already but the implementation is really "ugly" and a more rather quick solution to my needs now. Keep in mind I am fairly new to this Project, so I was a little bit careful in changing existing stuff to not break things. Just in the middle of writing the Issue, I saw that you already have seen my fork and left a comment there. **Customizations requests:** 1. Please could you make settings to make the CppSDK more customizable? I e.g. dont want to have all the .cpp files in the same directory than the .hpp files. I added a rather ugly [way](https://github.com/Zvendson/PalServer-Dumper/commit/161ab62de4d62aab1b080038836b43039029cd88) to make an `include` and `source` folder but it fits my project where I plan to use the dumped SDK and i can simply copy&paste the CppSDK folder (which i renamed too). Also neat would be to rename the SDK.hpp to a custom name too. 2. I added a [way](https://github.com/Zvendson/PalServer-Dumper/commit/9eac1d0379836249beedfaa02d4c01f014e45497) to take the current path of the .dll to generate the dump in the same directory, instead of using `C:/...`. It would be neat if that could be customizable. I dont want to create a PR for that, because you see my changes? I wouldnt want to have them too haha. 3. More infos on the settings itself. Me as someone who stumbled over this has not much experience what certain settings will change on the output and why that would be useful. 4. This is more like a question or 50% question. Is the dumped output usable for linux machines? To clarify. I have no exp with linux c++ but I want to change that sooner or later during my API development for the PalServer, to support native Linux servers without having them to install wine or proton. If not usable for linux, could there be a future change? 5. Would it be possible to add a kind of "filter" into the dumper? What I am talking about is that the PalServer, contains classes that are not used at all, bcs they are obviously for the gameclient. classes and functions that are e.g. for the UserInterface. I would like to append a list of strings that will prevent those classes to be generated at all. If thats not too hard to implement ofc. (Still getting overwhelmed to try to dig deeper into the dumper but I want to focus more on project that will depend on the dumper. So I am asking you here.) **Warnings:** *Disclaimer: I have been able to fix all warnings on /W3 for now but most changes are more ugly to achieve it.* 1. While fixing all the errors I stumbled about a "problem" that repeats itself almost everywhere: type conversations. I have several Ideas on how to change that overall: - Avoid using smaller type sizes for integers and use i64 "only". *Probably not the desired way but some code pieces def. should have a bigger size.* Common cases I saw where `for int i loops`. the index var is an int but the condition is compared to an unsigned value. [Example](https://github.com/Encryqed/Dumper-7/blob/main/Dumper/ObjectArray.cpp#L255) and here the [Fixed Version](https://github.com/Zvendson/PalServer-Dumper/blob/main/Dumper/ObjectArray.cpp#L255) - place the static_casts at the correct positions where you are sure that the is no dataloss possible. [Example](https://github.com/Zvendson/PalServer-Dumper/commit/c351c99f5f120f7aa0f3aa93472ca9142f9ef7f8#diff-627bb833aab47d37881066a00093a42b041a3cd1ce2d5c67cc0445f01511b084R293) - Dealing correctly with [wstring-string conversation:](https://github.com/Zvendson/PalServer-Dumper/commit/c351c99f5f120f7aa0f3aa93472ca9142f9ef7f8#diff-837fa120a2f0a9da78df502e51185757506e07379f4a30463997f3563af43c53R289). Not saying my implementation is the best but i would claim it decent enough. 2. Fix generated enum classes. While using the dumper, i stumbled across a lot of warnings that exceed the maximum size the underlaying enum type can actually hold, which results into a massive spam of overflow warnings. My solution is really bad for this and is a more or less frequent solution. I added enough comments for you to follow my [thoughts](https://github.com/Zvendson/PalServer-Dumper/commit/4b4f0bcbd7b46dd22a811ce8c5064eeb45f0c7a5) I add an example enum to emphasize more what I am talking about: ```cpp // Enum Engine.EStreamingSourcePriority // NumValues: 0x0007 enum class EStreamingSourcePriority : uint8 { Highest = 0, High = 64, Normal = 128, Low = 192, Lowest = 255, Default = 128, EStreamingSourcePriority_MAX = 256, // Warning bcs 255 is max size }; ``` my current solution writes the _MAX overflow beneath it as an `inline constexpr`. But i am not sure if the _MAX fields is actually used by the engine or required to be inside the enum. another thought was changing the enum class type to a bigger size but that could cause to misalignments? 3. Fixing the FVector. Im not sure if the "float" is required in every FVector implementation. I changed it to keep the `using UnderlayingType = double` [support](https://github.com/Zvendson/PalServer-Dumper/commit/4b4f0bcbd7b46dd22a811ce8c5064eeb45f0c7a5#diff-0d5dea9d33a2f94ee5fb29a954c60566cecad92f9ae822288ead0ef0d4b98cc9L2570). I tested the code by compiling with `using UnderlayingType = float` and it did not cause a warning. 4. class / struct mismatch. This one was the hardest to deal with. Basically the problem is: the dumper defines a class and later references this class as struct type. Those mostly occured with: `FProperty`, `FField` and `FStructProperty` when they are used in fields like: `TFieldPath<struct FProperty>` Here and example: ```cpp struct FPropertyAccessSegment final { public: class FName Name; // 0x0000(0x0008)(ZeroConstructor, IsPlainOldData, NoDestructor, HasGetValueTypeHash, NativeAccessSpecifierPrivate) class UStruct* Struct; // 0x0008(0x0008)(ZeroConstructor, NoDestructor, UObjectWrapper, HasGetValueTypeHash, NativeAccessSpecifierPrivate) TFieldPath<struct FProperty> Property; // 0x0010(0x0020)(HasGetValueTypeHash, NativeAccessSpecifierPrivate) class UFunction* Function; // 0x0030(0x0008)(ZeroConstructor, NoDestructor, UObjectWrapper, HasGetValueTypeHash, NativeAccessSpecifierPrivate) int32 ArrayIndex; // 0x0038(0x0004)(ZeroConstructor, IsPlainOldData, NoDestructor, HasGetValueTypeHash, NativeAccessSpecifierPrivate) uint16 Flags; // 0x003C(0x0002)(ZeroConstructor, IsPlainOldData, NoDestructor, HasGetValueTypeHash, NativeAccessSpecifierPrivate) uint8 Pad_3E[0x2]; // 0x003E(0x0002)(Fixing Struct Size After Last Property [ Dumper-7 ]) }; ``` My fix for that is extremely shit but I gave up to find out how i can check if the `Member`-param in ```cpp CppGenerator::GetMemberTypeStringWithoutConst(UEProperty Member, int32 PackageIndex) ``` is a class type. I could not figure that one out. so i was hoping you have a better solution for that. my "[Fix](https://github.com/Zvendson/PalServer-Dumper/commit/994be1a70ac3f3a316cd003590311b03975119bf#diff-0d5dea9d33a2f94ee5fb29a954c60566cecad92f9ae822288ead0ef0d4b98cc9L1130)" **Last Words:** I hope I did not forget anything here to mention. The most important things for me are that all the warnings are removed properly. Sorry for my OCD but i **must** obey them xD The Dumper is really amazing and does the Job really well. I really appreciate all the work that has been done here. I made the Fork to fit my needs but also as an Example where I could tackle the problems and give you a faster insight. I also made it a solution to fill this Issue with examples and provide a detailed report/request. I hope you have a great day/weekend!

❤️ 1

Fischsalat commented

2024-10-06 16:18:34 +02:00

(Migrated from github.com)

Customization Requests:

Please could you make settings to make the CppSDK more customizable?

I haven't been asked for that yet and it's certainly not impossible.
a way to take the current path of the .dll

That could probably be done in a nicer fashion with std::filesystme, and if not it should be an option in Settings::CppGenerator
More infos on the settings itself.

From my point of view all of the settings are pretty much self explanatory and a quick description is provided in the comment above. Do you have any example of what is unclear?
Linux
1. Is the dumped output usable for linux machines?
  
  No, right now the entire SDK generator assumes the availability of the windows api, and pretty much all of the Offset-finder code (for GObjects, GNames, AppendString, GWorld, ProcessEvent) assumes that the dumper is running on an x64 machine. In theory that portion should be portable to linux, but it contains too many windows specific things. For example, it heavily uses windows structs for sections, imports and memory validity.
2. If not usable for linux, could there be a future change?
  
  In theory yes, but it would be a lot of work. There would need to be different memory interfaces and different offset-finders per system depending on what information is available, what the instruction-set of the underlaying processor is and what other information is available.
Would it be possible to add a kind of "filter" into the dumper?
append a list of strings that will prevent those classes to be generated

Filters for some base-classes to exclude should be possible, but I'm not sure how much fun that would be in terms of dependencies. Because what if a non-excluded class contains a pointer to an excluded class? Keep the excluded class? Well, then I'd also need to keep the base class of the excluded class and all of the member the excluded class contained. Or references to the excluded class could be replaced by a void* or padding, but that would still be kinda weird to implement.
I think a feature like this requires a lot more consideration than I want to do right now.

Warnings

Fixing integer overflow/underflow warnings
Using uint64_t everywhere is kinda shitty. static_casts do the job, at least when it's not desirable to switch the type of the variable to a more fitting type. The conversion from wchar_t strings to char strings was done incorrectly (and stills is on main), but is fixed on the Unicode-Names branch. The reason this branch hasn't been merged back into main is that the name-validation (to filter out invalid names) doesn't properly support utf8 yet. I've written a library (unfinshed) that handles different encodings properly that will be used once ready.
Extending the size on enums is not a solution as it needs to match the game. Removing the _MAX value seems like a sufficient solution. I've never seen anyone use that anyways. A setting can be added Settings::CppGenerator::bKeepEnumMAXValues = false.
Nope, there is no reason to use float when UnderlayingType exists, it was just added later on and not all occurrences of float were replaced.
I'm not sure if I was moved to a parallel universe, but I'm pretty sure I fixed the class/struct missmatch 3 times already, yet somehow never did.

On your last words:

Issues with requests like this are welcome, at least if they are properly formulated like yours. Removing the warnings is kinda cool and I don't see any strong reason against it. I'm always happy when other people contribute to this project.

Feel free to open pull requests for all kinds of different features and fixes. If there's any issue with code quality, formatting or anything else I will let you know.

Also feel free to message me on discord (fischsalat) on anything, for a faster communication.

## Customization Requests: 1. > Please could you make settings to make the CppSDK more customizable? I haven't been asked for that yet and it's certainly not impossible. 2. > a way to take the current path of the .dll That could probably be done in a nicer fashion with std::filesystme, and if not it should be an option in `Settings::CppGenerator` 3. > More infos on the settings itself. From my point of view all of the settings are pretty much self explanatory and a quick description is provided in the comment above. Do you have any example of what is unclear? 4. Linux 1. > Is the dumped output usable for linux machines? No, right now the entire SDK generator assumes the availability of the windows api, and pretty much all of the Offset-finder code (for GObjects, GNames, AppendString, GWorld, ProcessEvent) assumes that the dumper is running on an x64 machine. In theory that portion should be portable to linux, but it contains too many windows specific things. For example, it heavily uses windows structs for sections, imports and memory validity. 2. > If not usable for linux, could there be a future change? In theory yes, but it would be a lot of work. There would need to be different memory interfaces and different offset-finders per system depending on what information is available, what the instruction-set of the underlaying processor is and what other information is available. 5. > Would it be possible to add a kind of "filter" into the dumper? > append a list of strings that will prevent those classes to be generated Filters for some base-classes to exclude should be possible, but I'm not sure how much fun that would be in terms of dependencies. Because what if a non-excluded class contains a pointer to an excluded class? Keep the excluded class? Well, then I'd also need to keep the base class of the excluded class and all of the member the excluded class contained. Or references to the excluded class could be replaced by a `void*` or padding, but that would still be kinda weird to implement. I think a feature like this requires a lot more consideration than I want to do right now. ## Warnings 1. Fixing integer overflow/underflow warnings Using uint64_t everywhere is kinda shitty. static_casts do the job, at least when it's not desirable to switch the type of the variable to a more fitting type. The conversion from `wchar_t` strings to `char` strings was done incorrectly (and stills is on main), but is fixed on the Unicode-Names branch. The reason this branch hasn't been merged back into main is that the name-validation (to filter out invalid names) doesn't properly support utf8 yet. I've written a library (unfinshed) that handles different encodings properly that will be used once ready. 2. Extending the size on enums is not a solution as it **needs** to match the game. Removing the `_MAX` value seems like a sufficient solution. I've never seen anyone use that anyways. A setting can be added `Settings::CppGenerator::bKeepEnumMAXValues = false`. 4. Nope, there is no reason to use `float` when `UnderlayingType` exists, it was just added later on and not all occurrences of `float` were replaced. 6. I'm not sure if I was moved to a parallel universe, but I'm pretty sure I fixed the class/struct missmatch 3 times already, yet somehow never did. ### On your last words: Issues with requests like this are welcome, at least if they are properly formulated like yours. Removing the warnings is kinda cool and I don't see any strong reason against it. I'm always happy when other people contribute to this project. Feel free to open pull requests for all kinds of different features and fixes. If there's any issue with code quality, formatting or anything else I will let you know. Also feel free to message me on discord (`fischsalat`) on anything, for a faster communication.

❤️ 2