dhewm
Refactor idGameEdit
Split idGameEdit into base class idGameEditBase which is in the main binary and idGameEdit which is only referenced in the game libraries.
This was an attempt to fix UBSAN compilation. The main binary now compiles with UBSAN but the game library does not. It requires vtables of things which only exist in the main binary. I think the way to make it work will be to implement static linking of game library. So this might not even be necessary. Thoughts?
Sorry, somehow forgot about this.
TBH, if this doesn't really fix the UBSAN problems (as the game lib still can't be built) I don't really see the point of this change.
Implementing static linking of the game lib seems like the way to go, and shouldn't be too hard (see d3wasm) - though in case of dhewm3 it of course needs to be optional (enabled by a CMake option that's off by default - ideally we could switch between "off" / "statically link base" / " statically link d3xp").
Well there's also the FIXME comment (that I seem to have forgotten to remove) that says this should be done.
@HarrievG do you want to incorporate this into your changes?
@HarrievG do you want to incorporate this into your changes?
Please don't - this breaks the Game API (=> makes mod DLLs incompatible), without solving any problem (that old FIXME isn't worth it)
If we break the game API in the future for some other reason we could think about incorporating this change
I'll check it out later this night or this week, will rebase, and make it ready.
On Thu, Apr 22, 2021, 15:54 Daniel Gibson @.***> wrote:
As far as I see #399 doesn't touch Game.h
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dhewm/dhewm3/pull/369#issuecomment-824862329, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIUHNXFZWUJL2HTQYOLARDTKATBBANCNFSM43BDSBYA .
"As far as I see #339 doesn't touch Game.h" The debugger changes are not public, but stashed locally. When Daniel has approved the 4k fixes pr, i am going to unstash my debugger changes and merge it with this one and make a seperate pr
"If we break the game API in the future for some other reason we could think about incorporating this change" If i recall correctly; i did not change the Game API, i only changed the Editor API so the debugger hooks could be exposed without adding deps - edit: hmm i think this actually comes down to the same thing, lets see when i unstash -
When Daniel has approved the 4k fixes pr, i am going to unstash my debugger changes and merge it with this one and make a seperate pr
Well, we are waiting :D
@gl0527 This PR was just about splitting up the class so it stops breaking UBSAN. If you have improvement suggestions make them over at #379.