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

filesystem: introduce new module, based on engine filesystem. #914

Merged
merged 3 commits into from Jul 28, 2022
Merged

Conversation

a1batross
Copy link
Member

@a1batross a1batross commented Jul 1, 2022

The goal is to share filesystem code between engine and utilities and provide C++ VFileSystem interface in the future.

Does not link now, requires all exports to be set.

Next stage is to have dynamic C API (or not?)

@nekonomicon
Copy link
Member

It seems you forget to remove miniz.h from engine/common:
https://github.com/FWGS/xash3d-fwgs/blob/fs/engine/common/miniz.h

@a1batross
Copy link
Member Author

As suggested by @mittorn, should it be actually named "filesystem_stdio"?

  1. I want to mods use the same C++ interface to access filesystem in both GoldSrc and Xash3D FWGS, therefore it also has the same name.
  2. But on the other hand, this can be confusing as some users may accidentally replace our reimplementation with Valve's original library. Without some complicated runtime detection, the engine will refuse to load with some useless message.
    2.1. This could be avoided if the engine used the same C++ interface, also we could test for bugs this way. But I don't want to bring C++ to the engine.
    2.2. We also can just pretend that such users get what they deserve? I mean, we never asked users to override anything, except just copying gamedirs.

cc @SNMetamorph @nekonomicon

@SNMetamorph
Copy link
Member

As suggested by @mittorn, should it be actually named "filesystem_stdio"?

I think it's better to name it "filesystem" to avoid confusing. It looks like a more flexible solution - we don't stick to Valve's interface between FS and engine, therefore it can be expanded if it'll be needed.

@a1batross
Copy link
Member Author

@SNMetamorph but then what we do for compatibility? My goal is to make zero changes in existing mods.

Make a simple wrapper between extended interface and VFileSystem009?

@a1batross
Copy link
Member Author

a1batross commented Jul 4, 2022

@SNMetamorph for extended interface, we can make a new interface, since CreateInterface just returns a pointer by it's name.

@a1batross
Copy link
Member Author

a1batross commented Jul 4, 2022

The interface between engine and filesystem module is simple.

It's just a bunch of exported functions that you can link with (see filesystem.h). It's never meant to be stable or used by anything else than engine and utils. It's optionally possible to override memory allocation and log output functions.

The interface for games is planned to be only VFileSystem009.

@nekonomicon
Copy link
Member

Any yet another wrapper can be replaced by user too.
Ideally will be better to have compatibility with filesystem_stdio.
And I think we can just link statically filesystem module to engine and to utils plus provide wrapper for mods. Or there something wrong?

@a1batross
Copy link
Member Author

a1batross commented Jul 4, 2022 via email

@a1batross
Copy link
Member Author

Ideally will be better to have compatibility with filesystem_stdio.

That would mean that we have to change how engine works too.

Original filesystem_stdio doesn't have ZIP support or RoDir. Why should we be compatible with inferior, proprietary software just because some dumb ass user replaced the library?

@nekonomicon
Copy link
Member

Need to say "%USERNAME% is dumb ass" then if filesystem_stdio module was replaced :)

The goal is to share filesystem code between engine and
utilities and provide C++ VFileSystem interface in the future
@a1batross a1batross changed the base branch from master to 16bit July 26, 2022 08:55
@a1batross a1batross changed the base branch from 16bit to master July 26, 2022 08:55
@a1batross
Copy link
Member Author

With VFileSystem009 implementation, halflife-updated finally starts to work.

@a1batross
Copy link
Member Author

Let's merge it and see what it breaks later.

As anybody who I asked to review this code, haven't responded or found anything suspicious.

@a1batross a1batross merged commit 55a29e6 into master Jul 28, 2022
@a1batross a1batross deleted the fs branch July 28, 2022 15:56
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