Conversation
Addeec constexpr to defined constants. FIxed warning C5055 after enum changes.
Fixed wasrning on unintialized variables. Templated clrmem() macro
moved include to top of file.
Removed duplicate enum def.
Changes macros to constexpr/enums in util.h
Fixed / changed unneeded assignment.
|
#define GRAPH_VERSION (int)16 // !!!increment this whever graph/node/link classes change, to obsolesce older disk files. |
I would just keep that macro since it's part of the SDK |
More wondering if we should increment the value to 17 but since it's sdk guess it doesn't really matter. |
SaintWish
left a comment
SaintWish
left a comment
There was a problem hiding this comment.
Can you revert the macro changes in the src/public and src/engine files because we should avoid making macro changes to the engine SDK header files.
We should avoid making changes to the thirdparty/angelscript, since it's a thirdparty library source we should avoid making changes to it.
re-added typedef for vec3_t into vector.h
This reverts commit b792854.
Added using statement for vec3_t / Vector equivalence.
…c sdk files. 3 int in progdefs.h and 3 ints in studio.h changed to unsigned int.
|
Agreed re leaving SDK code alone.
The GRAPH_VERSION comment is just from Valve - the .nod files are v16 and
there's no reason to change it. I think they made changes without version
bumps too (maybe Day One has v16 .nod files that don't match the retail
format).
…On Thu, 26 Mar 2026 at 13:10, Saint Wish ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Can you revert the macro changes in the src/public and src/engine files
because we should avoid making macro changes to the engine SDK header files.
We should avoid making changes to the thirdparty/angelscript, since it's
a thirdparty library source we should avoid making changes to it.
—
Reply to this email directly, view it on GitHub
<#369?email_source=notifications&email_token=AA34IYWP5ITQ4HZDLRKSTV34SSNRRA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIMBRGEYTKMZRGEY2M4TFMFZW63VKON2WE43DOJUWEZLEUVSXMZLOOS6XA4S7OJSXM2LFO5PW433UNFTGSY3BORUW63TTL5RWY2LDNM#pullrequestreview-4011153111>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA34IYR4YPN5DITKGNE5LET4SSNRRAVCNFSM6AAAAACW4YQD6KVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHM2DAMJRGE2TGMJRGE>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
If the files need to be 1:1 the same I can remove the using vec3_t added and move back to a define in the vector.h header. |
|
Ah yeah vec3_t and Vector is a mess from memory - different on client and
server.
Do what you need to I reckon.
…On Thu, 26 Mar 2026 at 17:13, Rederick5 ***@***.***> wrote:
*Rederick5* left a comment (MSRevive/MasterSwordRebirth#369)
<#369?email_source=notifications&email_token=AA34IYX2HRLEA24XKHL5EL34STKCXA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIMJTGIZDINRXGA3KM4TFMFZW63VHMNXW23LFNZ2KKZLWMVXHJNLQOJPWG33NNVSW45C7N5YGK3S7MNWGSY3L#issuecomment-4132246706>
If the files need to be 1:1 the same I can remove the using vec3_t added
and move back to a define in the vector.h header.
There's some comparisons between objects in the sdk which flag
signed/unsigned mismatch warning. 6 ints changed in the last commit. if I
know which side of the comparison can safely be casted, I can keep these as
vanilla and just static cast in the comparisons that they are referenced.
It's about 90 some warnings but would prefer not to pragma disable it.
—
Reply to this email directly, view it on GitHub
<#369?email_source=notifications&email_token=AA34IYX2HRLEA24XKHL5EL34STKCXA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIMJTGIZDINRXGA3KM4TFMFZW63VHMNXW23LFNZ2KKZLWMVXHJNLQOJPWG33NNVSW45C7N5YGK3S7MNWGSY3L#issuecomment-4132246706>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA34IYQQETTO2BHBE2OQ4VT4STKCXAVCNFSM6AAAAACW4YQD6KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DCMZSGI2DMNZQGY>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
Looks like changing 3 of the sdk files to vec3_t will cause some problems with this PR that will take time to resolve. Might just be a few cpp files missing the appropriate includes, but not sure why it would work as a straight specification but fail when using a macro define unless it's just how the linker is processing. I'm pretty green when it comes to working with any linker issues. the using vec3_t = Vector at the top of: eiface.h or leaving the vec3_t as Vector allows for compile. If absolutely needed to leave these as vanilla I'll probably need to wade back into the files and ensure vector is being included appropriately. I know I changed a few of the includes when removing defines caused redefinition problems. |
|
You can just leave the Vector then if it'll compile. |
(should probably be ext_dll.h?)
|
Was able to find the include issue in animation.cpp. the public files should now all be base and the signed/unsigned mismatch warnings resolved. Also reverted changes to the two third party files. |
SaintWish
left a comment
SaintWish
left a comment
There was a problem hiding this comment.
Can you also revert the macro changes in src/common since those are also engine header files?
Missing M_PI def for a few files. Re-added nanmask int. Added hard define for missing enums, 'unable to open header file' and it's commented out of another common file.
|
That one hurts a bit, quite a few hours worth, but it has been done, with a few quick fixes to fill in missing defines. |
Nope, I think everything else is good. |
|
Since everything looks good and compiles, I'll probably accept it next week sometime. |
3 participants
This is partial completion. Changed a bit more than I probably should have, but standards indicate that changing to typesafe non-macros should be a good end goal, should it be able to be done safely.
For now, left a lot of the more complex macros, such as the loggers/message and engine callbacks alone.
Changed as many macro functions to class member/file functions / unrolled macros where possible.
Left thirdparty/public/engine files alone for the most part.
Changed all the vec3_t to Vector class, removed all #define constants I could with constexpr/enum where possible.
Resolved a few header inclusion issues / duplicated defines.
changed vector class to be able to be initialized as constexpr class objects.
I can compile without issue and some light testing indicates general stability, but comprehensive testing needs to be completed.
Script events in edana have tested successfully, etc.
FALSE is still cast as int
TRUE is still defined as int
NULL is still defines as int
future issue recommended to change these to true/false/nullptr/0 where applicable