-
Notifications
You must be signed in to change notification settings - Fork 219
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
Request: Remove warnings and add more customization please #246
Comments
Customization Requests:
Warnings
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 ( |
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:
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
andsource
folder but it fits myproject 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.
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.
experience what certain settings will change on the output and why that would be useful.
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?
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.
I have several Ideas on how to change that overall:
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
implementation is the best but i would claim it decent enough.
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:
inline constexpr
. But i am not sure if the _MAX fieldsis 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?
using UnderlayingType = double
support.I tested the code by compiling with
using UnderlayingType = float
and it did not cause a warning.the dumper defines a class and later references this class as struct type. Those mostly occured with:
FProperty
,FField
andFStructProperty
when they are used in fields like:TFieldPath<struct FProperty>
Here and example:
Member
-param inCppGenerator::GetMemberTypeStringWithoutConst(UEProperty Member, int32 PackageIndex)
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!
The text was updated successfully, but these errors were encountered: