YaLTeR / OpenAG

An open-source reimplementation of Adrenaline Gamer's client library.
https://j.mp/OpenAG
Other
131 stars 42 forks source link

Small additions and cleanup #175

Closed SmileyAG closed 8 months ago

SmileyAG commented 8 months ago

@tmp64, I got noticed that BHL-Rebased repo is also utilizes the com_model.h version for software engine that is shipped by Valve in the HLSDK repo and you haven't a few undocumented functions in the end of interfaces, so you might to update it as well

Changes (e.g. com_model.h) is not Xash stuff or whatever you might got some wrong thought, it got clearly reverse-engineered from linux version of engine and even compared with Quake source code for a case (but it doesn't have any Quake-only comments or name variables, otherwise it would be violate due of GPL license, even if some names or comments are the same, then this is only because it also in the HLSDK / Source SDK / GoldSrc decompiled code, and Valve not take down the RE or leaked code generally speaking in compare to other companies, so there is no point for clean-room design)

tmp64 commented 8 months ago

Since you're cleaning up, you might as well replace the copyright symbol with (c) or convert it to UTF-8

SmileyAG commented 8 months ago

Since you're cleaning up, you might as well replace the copyright symbol with (c) or convert it to UTF-8

Well that sounds good, since that wouldn't mess each time if contributor using different encoding from Windows-1252 or ISO-8859-1 But the issue is that would create a new commit as latest in most of files (169 files would edited from that), which personally I don't like to have it, since it really disturb from tracking latest change of each file

@YaLTeR Might then if I make a new commit with replacing copyright symbol to (c) and that pull request would be merged, you would run git rebase --interactive 5d761709a31ce1e71488f2668321de05f791b405 --committer-date-is-author-date and move the commit (that with replacing copyright symbol) to the place right after 5d761709a31ce1e71488f2668321de05f791b405 commit?

From merge conflicts there is only in StudioModelRenderer.h file, that argue on copyright symbol for a only like two commits, so that a few seconds to resolve

Or rather not make that copyright symbol change at all then and keep as it now

YaLTeR commented 8 months ago

Matched structs in com_model.h with hardware and software engine versions (software additions could be enabled with set of SOFTWARE_BUILD preprocessor flag)

But software is a runtime setting and you can't ship two different client DLLs for software and hw, can you? How does this work in the official mods?

Or rather not make that copyright symbol change at all then and keep as it now

Yeah let's just not bother for now.

SmileyAG commented 8 months ago

But software is a runtime setting and you can't ship two different client DLLs for software and hw, can you? How does this work in the official mods?

Well, there is two variations of the engine: hardware (hw.dll, hw.so) and software (sw.dll, sw.so), in which the model structures are different If you need to maintain compatibility between them, you can compile the client-side by default for using model structures as in hardware engine, or by setting the preprocessor flag to compile as it in software engine

Half-Life SDK code and most modders probably haven’t gone that far in using them, so it doesn’t get to the point where your game crashes or even worse: UB

But those who used it quickly realized that the com_model.h that Valve provided to them was incorrect and took these structures directly from Quake headers for the hardware engine

For example, source code from Paranoia did this back in 2007: https://github.com/HLSources/Paranoia/blob/master/code_src/common/com_model.h (and let me remind that the first versions were made specifically on the GoldSrc engine, support for Xash3D appeared later)

They were just lucky that the beginning and middle of most of these structures are exactly the same as in Quake headers, although this of course does not change the fact that the size of the structures can be different and this can also cause issue depending on the case (e.g. if Valve added some new variable at the end of struct)

YaLTeR commented 8 months ago

So nobody actually handles this? Since users will only download one DLL (the hardware one).

SmileyAG commented 8 months ago

So nobody actually handles this? Since users will only download one DLL (the hardware one).

Kinda. I'd say it fair to release only hardware version of client, since the majority of users run the goldsrc games in hardware mode, but leaving opportunity to compile code specifially to support the software engine is nice option too

YaLTeR commented 8 months ago

Alright I guess, as implemented now it doesn't really hurt.

YaLTeR commented 8 months ago

Thanks