Skip to content
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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add Particleman #444

wants to merge 4 commits into from

Conversation

FreeSlave
Copy link
Member

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 with USE_PARTICLEMAN option to make the particleman available.

Also I made particles reset in the HUD_VidInit instead of MsgFunc_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).

@a1batross
Copy link
Member

I don't think it really needed for portable SDK.

Not unless we ship particleman sources as well.

@a1batross
Copy link
Member

Not only that, loading separate DLLs breaks library suffixes.

If USE_PARTICLEMAN is set, libdl also must be linked on Linux.

@FreeSlave
Copy link
Member Author

FreeSlave commented Mar 13, 2024

Particleman is loaded via the GoldSource module interface. The client itself doesn't need to be linked against libdl. I just checked the build without GoldSource support enabled so the produced client library doesn't depend on the libdl.

Upd: I rechecked the code and it does use dlopen, but somehow it still works even without linking to libdl. I guess the it just works because the engine is linked to it.

@a1batross
Copy link
Member

Yeah, linker makes all symbols loaded by main executable visible.

But it's still technically a bug, it's better explicitly link to libdl.

@a1batross
Copy link
Member

Not unless we ship particleman sources as well.

btw if somebody steps in and reverse engineer particleman, we would be able to link it statically.

@FreeSlave
Copy link
Member Author

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.

@a1batross
Copy link
Member

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.

@a1batross
Copy link
Member

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.

@Velaron
Copy link
Member

Velaron commented Mar 13, 2024

https://github.com/Velaron/particleman/tree/dev
I have one but sadly it's like only 90% done for now

@FreeSlave
Copy link
Member Author

@a1batross is there really something wrong with STL usage though?

@a1batross
Copy link
Member

@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.

@nekonomicon
Copy link
Member

nekonomicon commented Mar 13, 2024

is there really something wrong with STL usage though?

Fat dependency and may be not portable.
You can replace it with miniutl or nanostl if possible.
Currently, we can use libsupc++ instead of libstdc++/libc++-rt/libgcc.

#include <stdio.h>
#include "interface.h"

#if !defined ( _WIN32 )
Copy link
Member

@nekonomicon nekonomicon Mar 13, 2024

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
Copy link
Member

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;
Copy link
Member

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
Copy link
Member

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@a1batross
Copy link
Member

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".

@FreeSlave
Copy link
Member Author

FreeSlave commented Mar 13, 2024

@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.
There's no point extensively working on it if mods don't use it.

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.

@nekonomicon
Copy link
Member

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.
There's no point extensively working on it if mods don't use it.

I undestand it, but will be good to make it in one style.

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.

Is it with statically linked libstdc++ and libgcc_s?

I have added such strings into readme:
Alternatively, you can avoid libstdc++/libgcc_s linking using small libsupc++ library and optimization build flags instead(Really just set Release build type and set C compiler as C++ compiler):

cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_COMPILER=cc -B build -S .
cmake --build build

@FreeSlave
Copy link
Member Author

Is it with statically linked libstdc++ and libgcc_s?

No, the same method that is used in our CI (build with steam runtime chroot).

@a1batross
Copy link
Member

a1batross commented Mar 13, 2024

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.

@a1batross
Copy link
Member

Let's not forget that this is hlsdk-**portable** and not some overly hacked SDK that doesn't give a crap about other engines, platforms and so on.

@a1batross
Copy link
Member

It's much more likely to have a platform with unimplemented dynamic library loader (which you do in public/interface.cpp), than not have a working STL.

@FreeSlave
Copy link
Member Author

I agree that having a particleman implementation as a part of the client library is better.
For now this PR is just about adding the same stuff that Valve's repo has.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants