-
Notifications
You must be signed in to change notification settings - Fork 126
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
Add Particleman #444
base: master
Are you sure you want to change the base?
Add Particleman #444
Conversation
I don't think it really needed for Not unless we ship particleman sources as well. |
Not only that, loading separate DLLs breaks library suffixes. If |
Particleman is loaded via the GoldSource module interface. The client itself doesn't need to be linked against Upd: I rechecked the code and it does use |
Yeah, linker makes all symbols loaded by main executable visible. But it's still technically a bug, it's better explicitly link to |
btw if somebody steps in and reverse engineer particleman, we would be able to link it statically. |
There's reimplementation by SoloKiller which I included in my sdk. It requires C++17 though (I decreased the requirement to C++11). I also saw some alternative reimplementation (using hand-written lists instead of STL containers). I think it was here https://github.com/whamemer/particleman but the repo is not available anymore. |
https://github.com/2010kohtep/OpenHL-Particleman There is this, but it's GPL licensed. I don't think we should make the situation of potentially incompatible licenses mix worse than it is already. |
https://github.com/twhl-community/halflife-updated/tree/master/cl_dll/particleman It seems to be a viable alternative for now. We might modify to decrease STL usage, but it might be done separately later. |
https://github.com/Velaron/particleman/tree/dev |
@a1batross is there really something wrong with STL usage though? |
@FreeSlave I don't think there is really anything wrong with it, but if we use STL here, it would be nice to use it everywhere, for consistency. @mittorn seems to be against its usage. |
Fat dependency and may be not portable. |
#include <stdio.h> | ||
#include "interface.h" | ||
|
||
#if !defined ( _WIN32 ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use definitions from build.h
?
And convert all #ifdef/#if defined
to #if
?
//----------------------------------------------------------------------------- | ||
CreateInterfaceFn Sys_GetFactoryThis( void ) | ||
{ | ||
#ifdef LINUX |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, avoid such platform checks.
|
||
|
||
// Some helpful typedefs. | ||
typedef std::vector<MemoryBlock *> VectorOfMemoryBlocks; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
STL dependency here.
#ifndef PARTICLEMEM_H__ | ||
#define PARTICLEMEM_H__ | ||
|
||
#ifdef _WIN32 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Useless platform check here.
EXPOSE_SINGLE_INTERFACE_GLOBALVAR(className, interfaceName, versionName, __g_##className##_singleton) | ||
|
||
|
||
#ifdef WIN32 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather have a particleman impl added to this repo and linked statically, even with STL, instead of adding this whole "let's dlopen some random library". |
@nekonomicon I just copied files from Valve's repo, thus it has original macro checks and everything. The point is to let modders have a version of our sdk matching the Valve's one in terms of having particleman available. I don't understand how STL is "fat". Client library in my sdk before adding ParticleMan implementation is 595 KB in the relese build, and after I've added full ParticleMan reimplementation it's 619 KB. This branch is 451 KB vs master which is 447 KB. Recently I rewrote a part of my sdk to use STL (part of the subtitles system particulary), just for a test, and the resulting .so had the same size as with sort and binary search written by hand. |
I undestand it, but will be good to make it in one style.
Is it with statically linked libstdc++ and libgcc_s? I have added such strings into readme:
|
No, the same method that is used in our CI (build with steam runtime chroot). |
The thing is, not all our supported platforms might have a real STL library. Usually it's feature-complete libstdc++, Microsoft's STL or libc++. But in very rare cases it might be crippled libsupc++, horribly broken and not updated stlport, or nothing at all. Again, I said what I do think about this. It's much more important to have Particle Man implementation (because it automatically means we can just, you know, COMPILE it) and deal with all this STL hatred nonsense later. |
Let's not forget that this is |
It's much more likely to have a platform with unimplemented dynamic library loader (which you do in |
I agree that having a particleman implementation as a part of the client library is better. |
Half-Life itself doesn't use particleman, only Counter Strike and Day Of Defeat do. So I've added the
test_particles
command to test the particleman. The client must be built withUSE_PARTICLEMAN
option to make the particleman available.Also I made particles reset in the
HUD_VidInit
instead ofMsgFunc_InitHUD
(like it's done in the Valve's repo) to ensure the particles don't stay after the changelevel.This PR exists just in case anyone wants to use Particleman in their mod based on this sdk. It might be not useful to merge to master (again, because HL doesn't use particleman). If you decide it needs to be merged, it must be tested on non-GoldSoure platforms as well (i.e. it shouldn't cause build failing on Xash3D FWGS suported platforms).