New filesystem #76

Chomenor · 2018-04-15T20:17:29Z

This is my effort to port my ioquake3 filesystem project to Tremulous. It's not actually complete/ready to merge yet, but @wtfbbqhax suggested I open a pull request so it can be covered by the CI system.

My current progress is that the game is in a roughly working state, and it should be able to run and connect to most servers. However more advanced features like special pk3 precedence rules and support for hosting multi-protocol servers have not been implemented yet.

…urce dir count

…consistent with original filesystem

wtfbbqhax · 2018-04-16T00:03:55Z

@Chomenor Can you run clang-format on your new files; I provide a .clang-format at the root of the repo.

Another point on style; all engine code uses C++ true/false instead of qtrue/qfalse.

wtfbbqhax · 2018-05-01T22:11:24Z

@Chomenor great job on the file system, I've been giving it a test drive for a little bit. I'm going to follow full review shortly.

wtfbbqhax · 2018-05-01T22:12:46Z

src/client/cl_avi.cpp

@@ -608,7 +608,16 @@ bool CL_CloseAVI( void )
  // Write index

  // Open the temp index file
+#ifdef NEW_FILESYSTEM
+  indexSize = 0;
+  { char path[FS_MAX_PATH];


Regrettably, It's not documented anywhere and some files (such as this one) have not yet been updated, but please try to follow a general style of

indexSize = 0; { char path[FS_MAX_PATH]; if(fs_generate_path_writedir(FS_GetCurrentGameDir(), idxFileName, 0, FS_ALLOW_SLASH, path, sizeof(path))) afd.idxF = fs_direct_read_handle_open(0, path, (unsigned int *)&indexSize); }

Notably

Curly braces on a separate line (exceptions for small one line conditionals and functions)

4 space indent (no tabs)

wtfbbqhax · 2018-05-01T22:19:27Z

src/client/cl_cgame.cpp

            FS_FCloseFile( args[1] );
            return 0;
        case CG_FS_SEEK:
+#ifdef NEW_FILESYSTEM
+			if(fs_handle_get_owner(args[1]) != FS_HANDLEOWNER_CGAME) return 0;


Need to fix the indenting throughout here

wtfbbqhax · 2018-05-01T22:27:29Z

src/client/cl_main.cpp

            clc.cURLDisconnected = false;
            if (!clc.activeCURLNotGameRelated) CL_Reconnect_f();
            return;
        }
    }

+#ifndef NEW_FILESYSTEM


Not sure we need to ifndef the one line comment

I use a script to verify the ifdefs (no new filesystem changes outside them) so that's why I always use them. I expect you will probably remove the ifdefs altogether at some point if you end up using the filesystem.

wtfbbqhax · 2018-05-01T22:29:52Z

src/client/cl_main.cpp

@@ -1991,6 +2023,82 @@ A download completed or failed
 */
 void CL_NextDownload(void)
 {
+#ifdef NEW_FILESYSTEM
+	// Attempts to initiate a download, or calls CL_DownloadsComplete if no more downloads are available
+	*clc.downloadTempName = *clc.downloadName = 0;


s/0/'\0'/

wtfbbqhax · 2018-05-01T22:33:08Z

src/client/cl_main.cpp

@@ -4727,7 +4860,13 @@ static void CL_InitRef(void)
    ri.FS_WriteFile = FS_WriteFile;
    ri.FS_FreeFileList = FS_FreeFileList;
    ri.FS_ListFiles = FS_ListFiles;
+#ifdef NEW_FILESYSTEM
+	// This function is not implemented in the new filesystem, and it does
+	// not appear to actually be used in the renderer.


Feel free to cleanup the renderer api and remove it.

If I just delete the field it would cause a crash with mixed renderer versions, so I'm not sure I want to do that right away

There is no use case for client/renderer version mismatch. I expect this scenario to not work in general.

wtfbbqhax · 2018-05-02T00:00:28Z

src/filesystem/fs_misc.cpp

+    return qtrue;
+}
+
+int fs_get_source_dir_id(const fsc_file_t *file)


Consider making these member functions of fsc_file_t

wtfbbqhax · 2018-05-02T00:02:02Z

src/filesystem/fs_misc.cpp

+static qboolean inactive_mod_file_disabled(const fsc_file_t *file, int level)
+{
+    // Check if a file is disabled by inactive mod settings
+    if (level < 2)


Invert checks like this; if ( level >= 2 ) return false; keep the indentation of the rest of your code close to one level deep as possible.

wtfbbqhax · 2018-05-02T00:04:16Z

src/filesystem/fs_misc.cpp

+// Hash Table
+/* ******************************************************************************** */
+
+void fs_hashtable_initialize(fs_hashtable_t *hashtable, int bucket_count)


More of this I would want to be considered for replacement with a std::map vs a custom hash table.

wtfbbqhax · 2018-05-02T00:05:42Z

src/filesystem/fs_misc.cpp

+{
+    // Valid for an uninitialized hash table
+    hashtable->bucket_count = bucket_count;
+    hashtable->buckets = (fs_hashtable_entry_t **)Z_Malloc(sizeof(fs_hashtable_entry_t *) * bucket_count);


Z_Malloc and friends are garbage and I'd rather you use malloc. If nothing else, because my debug builds all use address sanitizer.

To elaborate on this; Z_Malloc prevents memory security devices (such as heap cookies), but also prevent tools like address sanitizer and valgrind.

wtfbbqhax · 2018-05-02T00:06:44Z

src/filesystem/fs_lookup.cpp

+} query_result_t;
+
+/* ******************************************************************************** */
+// Resource construction - Converts file or shader to lookup resource


can you elaborate, perhaps in some documentation what a "lookup resource" is?

It's a temporary structure that can represent either an image or shader, and contains all the values needed to sort the resource against other resources.

Chomenor · 2018-05-02T03:43:24Z

Regarding the code formatting and C++ integrations, it would be helpful to me to delay some of these changes until some other steps are accomplished. The main steps I would like to see are as follows:

Resolve the file layout questions from issue Proposal for a more userfriendly homepath/basepath #78, and get the basic structure of the pk3 precedence/fs_game handling implemented
Decide whether to use a new system based on pk3 manifests to handle multi-version pk3 support (in place of the old fs_pk3PrefixPairs system), and if so get a basic working implementation
Establish at least a provisional decision in favor of integrating this filesystem (or against)

Delaying more extraneous features and code changes until these major steps are addressed would help me with sharing certain work between this project and my ioq3 project, and also helps limit how much time I invest in this port before a decision has been made on whether you will actually end up using it.

dGr8LookinSparky · 2020-01-01T00:23:36Z

Hello, Merry Christmas, Happy New Year, and in general Happy Holidays @Chomenor ! I hope all is well. I apologize for this reply being so so so late (been busy with a lot of irl stuff, but now I have more time to work a lot more on this project again). I finally got around to fixing up that precedence order on this repo's wiki ( https://github.com/GrangerHub/tremulous/wiki/File-System-Load-Precedences ), and made some other misc edits to that wiki.

If you are still interested and have some time, I would greatly appreciate it if you update your work for trem based on the requested layout, and include your latest updates to your ioq3 implementation (I just reviewed the latest version of your file system readme in your ioq3 repo, and I really like all of the features you have listed there). After your file system is completed and it is tested well with tremulous clients and tremulous servers. I would really like to include your file system in an upcoming client release. If you can't work on this, I understand, and I would have time to try to finish your file system for trem starting about 2 months from now (before then I'm mainly focusing on getting the new game play, game mode system, and bot system done for 1.3 in the next 2 months). But if you would work on finishing your file system for our Tremulous project, that would be very awesome!

If you have any questions, feel free to ask!

dGr8LookinSparky · 2020-01-01T01:22:53Z

Some relatively new features/enhancements we've made that has some relation to the file system:

You can open the homepath, demos, and screenshots folder from main menu options ( 271734f )
Demo playback is fixed based on @Chomenor 's suggestion of having the client read the fs_game in the demo ( e4c259e ).

Chomenor · 2020-01-01T08:12:44Z

Sure, I'm willing to help finish the filesystem. I think it was fairly close to being completed before, at least to the extent that Tremulous could run and connect to servers.

I probably don't have time to develop every filesystem-related feature by myself, but if you can handle testing and most work that needs to be done outside the filesystem code, I should be able to handle most of whatever is needed within the filesystem.

Did you ever make progress on splitting the vm (game module) compilation off from the engine repository? If I remember right that was one of the last things we had been discussing.

dGr8LookinSparky · 2020-01-01T08:31:54Z

Awesome! Thanks @Chomenor , this is much appreciated :) ! I'll help where I can. I think I'm familiar enough with the engine code to figure out how to implement required stuff outside of the file system (although I can certainly become more familiar). I did compile a version with your file system awhile ago, and it did have the vast majority of the features working very well then. I personally only develop on Linux at the moment, but with whatever is left, most likely it should work on the supported Windows, and Mac platforms, and we can find players to test on those platforms.

@wtfbbqhax (aka blowFish) I think that did remove the vm compilation from this engine repo, I'm not 100% sure though. This engine repo still compiles the dynamic libraries for its included older game logic though.

The game logic repo does still build the cgame and ui qvm, although we disabled the game qvm support there, since it wasn't important for the purposes of that repo, and was a pain to keep functional with the game logic development. This engine repo and the game logic repo are not compatible with each other yet, the game logic repo has its own fork of the engine that has been modified for the server development, but after I finish the new game play, game mode system, and bot system, I'll be working on making the two repos compatible, and then strip the engine parts from the game logic repo, and strip the game logic parts from this engine repo, then probably make use of a submodule of the game logic repo in this engine repo.

With that said, the file system can probably be completed on this repo before/alongside any of those other planned updates without issue, and we may not even have to worry about porting this file system to the game logic repo, since ultimately the game logic in the game logic repo will be coming to this engine repo one way or another.

Chomenor · 2020-01-01T14:45:05Z

Based on a quick test the makefile in this engine repo does appear to generate qvm files.

What I envision is creating some "stock" qvm pk3s that are packaged with the engine, which would be used for the purposes of the main menu ui, connecting to unpure unmodded servers, and running a local game. I would suggest treating these pk3s more as assets than code, so they would potentially go in an asset repo and get copied into engine distributions from there, rather than built directly. That way the pk3 hashes remain stable for both the filesystem and potentially pure servers to reference.

So the repo layout would be as follows:

engine repo (does not build any qvms or game dlls)
stock game module repo (or maybe more than 1 if needed for multiple protocols for example)
asset repo which contains stock game module pk3s built from the game module repo

dGr8LookinSparky · 2020-01-01T20:07:10Z

@Chomenor , sounds like a generally good approach. There are a few things that I would like to add/suggest/ask.

I was thinking that we would also include hashes for the dynamic libraries as part of the hard coded "default content". We would have to have a different hash for each platform build, in which case that part of the "dynamic content" should do checks that would be based on the platform the engine was built for.
For a given dynamic library on a given platform, for a given qvm, and a given pk3 containing a given set of qvms, if the game logic code doesn't change, would all of the hashes be unchanged from multiple builds and subsequent packaging of the same code?
I estimate that it would be 2 to 3 months before the latest 1.3 game logic repo would be ready for use by this engine repo. Also the overall assets we have for the latest 1.3 game logic repo isn't entirely compatable with the game logic of this current engine repo (mainly regarding differences in the config files). But in the meantime, we could improvise by making a few new build scripts (with any required adjustments to the make file and the travis file), where one script would build and package just the game logic of this repo, and the other would build just the engine and finish off the process. Then I could make a temporary new repo that would be a copy of this repo, that we would have travis build just the game logic on that repo, and package all of the included assets on that repo, and then we could have travis on this current engine repo download the appropriate asset/gamelogic release from that other temporary repo whenever travis builds this current asset repo. Let me know if this looks like a good approach to you, then I'll get started with setting this up.
Regarding the different versions of the QVMs that would have to be built for the multiprotocol support. there just needs to be two versions of the cgame.qvm and of the ui.qvm, each in a separate pk3. gpp protocol clients and the latest protocol clients both can load the same cgame.qvm and ui.qvm, while 1.1 protocol clients require a seperate set of cgame.qvm and ui.qvm built for 1.1 clients.
Regarding the current arrangement of new and old default assets. In the case of 1.3 game logic, after the new cgame/ui/game for 1.3, and any new assets for 1.3 are loaded, it does use a good amount of the old default gpp assets, and a good amount of the old default 1.1 assets. Then for gpp servers, after the defualt gpp assets are loaded, it does use a good amount of the old default 1.1 assets. So (as I indicated in the wiki on the loading precedence https://github.com/GrangerHub/tremulous/wiki/File-System-Load-Precedences ) the hardcoded "default content" arrangement in the engine should take into consideration different major engine versions, where hardcoded "default content" of the most recent engine version for the given connected server is searched and loaded first, then the next older hardcoded "default content" engine version is searched and loaded, and so on until the 1.1 version is searched and loaded.

So for connecting to a 1.3 server: the precedence order of searching and loading the hardcoded "default content" would be as follows:

 - hardcoded 1.3 default content
 - hardcoded gpp default content
 - hardcoded 1.1 default content

For connecting to a gpp server: the precedence order of searching and loading the hardcoded "default content" would be as follows:

 - hardcoded gpp default content
 - hardcoded 1.1 default content

For connecting to a 1.1 server: the precedence order of searching and loading the hardcoded "default content" would be as follows:

 - hardcoded 1.1 default content

For connecting to a 1.4 server in the future: the precedence order of searching and loading the hardcoded "default content" would be as follows:

 - hardcoded 1.4 default content
 - hardcoded 1.3 default content
 - hardcoded gpp default content
 - hardcoded 1.1 default content

For connecting to a 1.5 server in the future: the precedence order of searching and loading the hardcoded "default content" would be as follows:

 - hardcoded 1.5 default content
 - hardcoded 1.4 default content
 - hardcoded 1.3 default content
 - hardcoded gpp default content
 - hardcoded 1.1 default content

and so on...

The pk3 files of the default assets and of the default QVMs for gpp and for 1.1 are available and will not change.

dGr8LookinSparky · 2020-01-01T21:04:53Z

Also, as a placeholder, I cold make a second temporary repo that would be a copy of this engine repo, but focused on just packaging the assests, and download the built game logic from the other mentioned temporary repo. so that the repo arrangement of this current engine repo, and two temporary placeholder repos (that would be copies of this repo), would behave similar to what you have described, @Chomenor .

Chomenor · 2020-01-01T23:31:59Z

Is it really necessary to use native code for the stock game modules? Using qvms only would simplify things a lot. I would assume qvms are generally better anyway (less chance of cross-platform issues, more commonality with mods used on actual servers). You can always load dlls manually if needed for debugging purposes.

Hashes would generally change between builds, hence the reason to store the game module pk3s as assets and only update them when necessary.

It doesn't matter what method or repo you use to create the stock module pk3s, as long as the actual saving of the pk3s is done manually. The important thing is the pk3 hashes are known in advance (before the engine is built) and don't change unless updated intentionally.

dGr8LookinSparky · 2020-01-02T00:32:23Z

QVMs have some advantages, in particular, as you've mentioned, a convenient way to distribute cross platform, and a relatively safe way to execute unknown code. However, we've found that they have a lot of disadvantages, they can break easily as we develop, especially when it comes to memory. I recently had to move all of the string literals used in the weapon/upgrade/buildable/class/team attributes structs to config files (although I was planning on doing that for the new game mode system anyways), because the QVM couldn't handle the stack memory used by those strings and would just crash. They are also significantly slower than dynamic libraries. There are other ways that they break that we had to work around as well. Additionally, the actual level of security from loading unknown QVMs, is questionable.

For the game logic in the 1.3 game logic repo, we just completely dropped the server side support for the game.qvm. The only reason we are keeping the QVM support for now is for backwards compatibility of old clients on the new servers (in regards to them loading the cgame.qvm and the ui.qvm), and for the new 1.3 clients to be able to play on old 1.1 and gpp servers. With the new config file based game mode system I'm working on, the vast majority of modding should be doable via the game mode config files without having to change the game logic, so mods would just have to distribute those config files in the vast majority of cases. Ultimately for what comes after 1.3, we are planning on dropping the use of QVMs, and have modding done entirely through the configs, and at some point we'll expand the Lua support, and even more modding could be done through Lua scripts.

With that said, I suppose that verification of the dynamic libraries as default content would not be necessary, since downloading of dynamic libraries from game servers is not allowed. perhaps as long as the dynamic libraries are in the correct version base folder for the given currently specified engine version, we could have them override the default content QVMs (that is while you are at the main menu, or download menu, or starting a local game that isn't set to use a non-base fs_game, or when connecting to a semi-pure or unpure server if following the overall precedence order would reuslt in them being loaded).

In other words, in the case that the 1.3 engine version is being used, in a situation where the dynamic libraries are allowed, and the active mod is the default base_1.3, first the dynamic libraries would be searched for and loaded in the base_1.3 folder, if they aren't found, then the hardcoded default content pk3 files with the default 1.3 QVMs would be searched for and loaded.

In the case that the gpp engine version is being used, and the active mod is the default gpp, first the dynamic libraries would be searched for and loaded in the gpp folder, if they aren't found, then the hardcoded default content pk3 files with the default gpp QVMs would be searched for and loaded.

Connecting to a 1.1 server that uses as its fs_game base, would work in the same way, but with respect to the base folder.

So it is just a matter of giving the dynamic libraries (if present in the proper version based folder) precedence over the corresponding QVM(s) located in the hardcoded default content pk3(s), without having to do any hash check on the dynamic libraries.

There is another potential issue I would like to bring up with the currently proposed design, that I'm hoping we can address. This design with hardcoded default content is perfect for standard client use, it ensures there is no funny business with attempts to override the main menu and download menu, and it comes with other advantages. But what about in the case where we are working on developing the default 1.3 game logic on a local setup, and we are testing using the 1.3 client and loading a local server through the client, and in addition connecting one or more other clients on the same local network for testing. Can that be done in a relatively straightforward automated way, with similar (or better) ease to how that can be done now with the old file system currently in use?

Chomenor · 2020-01-02T01:25:42Z

Is the requirement to use native dlls specific to the game module? Could we use qvms for the cgame and ui?

dGr8LookinSparky · 2020-01-02T02:01:20Z

Well, somewhat, for now. But I think we can handle the consideration of all dynamic libraries for when default content is loaded in a straightforward way entirely through the precedence search/load order without having to worry about checking the specific dynamic library is the correct one, other than checking that it is located in the right place. Here is what I'm thinking for this case:

search in the base_1.3/ folder for the dynamic libraries of the game, cgame, and ui, load any/all that are found there. Stop searching for the specific module(s) found, if found, only continue the next search/load for modules not found.
search everywhere for the pk3(s) specified as 1.3 default content via hardcoded hash that contain the game, cgame, and/or the ui that was not found in the previous dynamic library search, load those that are found, if any. Stop searching for the specific module(s) found, if found, only continue the next search/load for modules not found.
search in the gpp/ folder for the dynamic libraries of the game, cgame, and ui, load any/all that are found there. Stop searching for the specific module(s) found, if found, only continue the next search/load for modules not found.
search everywhere for the pk3(s) specified as gpp default content via hardcoded hash that contain the game, cgame, and/or the ui that was not found in the previous dynamic library search, load those that are found, if any. Stop searching for the specific module(s) found, if found, only continue the next search/load for modules not found.
search in the base/ folder for the dynamic libraries of the game, cgame, and ui, load any/all that are found there. Stop searching for the specific module(s) found, if found, only continue the next search/load for modules not found.
search everywhere for the pk3(s) specified as 1.1 default content via hardcoded hash that contain the game, cgame, and/or the ui that was not found in the previous dynamic library search, load those that are found, if any. Stop searching for the specific module(s) found, if found, only continue the next search/load for modules not found.

If connecting to a semipure or unpure server that is determined to be a 1.3 engine version, the default content search starts at search/load 1) listed above. If connecting to a semipure or unpure server that is determined to be a gpp engine version, the default content search starts at search/load 3) listed above. If connecting to a semipure or unpure server that is determined to be a 1.1 engine version, the default content search starts at search/load 5) listed above.

For now, if connecting to a pure server, skip the dynamic library search/load for the cgame and ui modules, but still include the dynamic library search/load for the game module in the case of starting a local game server through the client.

The engine version of the server you are connecting to can be determined by finding the highest engine version that includes in its hardcoded default content at least one pk3 file that the given connected server lists.

dGr8LookinSparky · 2020-03-06T06:22:02Z

@Chomenor , sure, I'll put something together sometime tomorrow (or later today rather, it's 1:21 am here at the moment). I think I'll copy the setup we have for the GrangerPub server, since that uses the tremded from this repo.

dGr8LookinSparky · 2020-03-06T21:18:19Z

@Chomenor , this is pretty much the same setup we have for the GrangerPub server, I tested this .zip on my local Linux computer, and it works: https://www.dropbox.com/s/9pitcb2ahtxf0rb/GPub_Setup.zip?dl=0

You are going to have to change the tremded repo in the update.sh script to your repo, and then run ./update.sh to automatically clone the required repos into the source/ folder, and everything would be built and placed into the correct folders. Feel free to adjust the scripts to fit your needs. config.sh contains a lot of the settings for this setup. run.sh is used to run the server without debugging (and automatically restarts the server if it crashes). Looking at the debug.sh script, it doesn't look like it does what it should do. The other bash scripts for the most part are for handling the game logs, probably not very important for your testing.

Chomenor · 2020-03-07T02:15:17Z

Looks good, thanks. I'll start working on it.

Since I'm planning to use a new system for specifying versioned pk3s, is it okay if I drop support for fs_pk3PrefixPairs? That reduces complexity but it does mean that existing server configs will need to be updated to use the new system.

dGr8LookinSparky · 2020-03-07T02:19:14Z

Sure, as long as the new system works for providing the multiprotocol support at least just as well now, I'm all for that. It probably wouldn't require too much alteration of the bash scripts, let me know if/when you need any assistance with adjusting any of the scripts.

Chomenor · 2020-03-29T04:32:39Z

@dGr8LookinSparky I've added basic multi-version pk3 support now. I haven't tested it much, but it does seem to work with the server config you gave me to the extent that different client versions could successfully connect.

I added a wiki page documenting the new system here. The example at the bottom shows the lines that need to be added to one of the server config files.

dGr8LookinSparky · 2020-03-29T11:17:54Z

Very awesome work @Chomenor :) ! I'll find some time in a few days to test it out, and perhaps setup a new test server.

…14.11

dGr8LookinSparky · 2020-03-30T13:27:11Z

Howdy @Chomenor , I found a bug where if after you build a release locally, if you delete the ui.so in the basemod/ folder so that the ui qvm is loaded in the vms pk3, the client infinitely fails to load something related to the ui. I'm not entirely sure if that is due to something I screwed up with updating the default assets, or if it was a bug that started with the one of the other recent updates/merges. I pushed what I have currently.

Chomenor · 2020-03-30T14:24:34Z

Sorry about the merge problem. I seem to be having some line ending related problems using Git on Windows since the .gitattributes was recently changed. Git's line ending behavior is really confusing to me. I wish it would just leave the line endings alone instead of changing them back and forth randomly...

I'm going to try to switch to git on Linux for interacting with this repository and hopefully that will stop the problems.

dGr8LookinSparky · 2020-03-30T14:28:38Z

yeah, we had to change the git attributes because it was corrupting new audio files at least when added from linux. Maybe it can be fixed up more to work for windows again (but without messing around with things like audio files)? This is one of those obscure aspects of git.

Chomenor · 2020-03-30T14:37:40Z

It seems like it is possible to tag certain extensions as binary, like

*.ogg binary
*.jpg binary

etc. I'm not sure that's related to my particular problem, but it could be useful.

cengique · 2020-03-30T14:48:14Z

@Chomenor you may need to clone again so that your line endings are now consistent. I agree that audio files should be marked binary, but to switch safely between Linux/Mac and Windows devs, we still have to use the auto switch. So the text=auto line in .gitattributes is the right solution, which corresponds to the autocrlf option in git config. However, I'm not sure if we're missing the core.eol=crlf in the file that is also implied. There's also a safecrlf option, that I haven't used before:

core.eol
           Sets the line ending type to use in the working directory for files that are marked as
           text (either by having the text attribute set, or by having text=auto and Git
           auto-detecting the contents as text). Alternatives are lf, crlf and native, which uses
           the platform’s native line ending. The default value is native. See gitattributes(5)
           for more information on end-of-line conversion. Note that this value is ignored if
           core.autocrlf is set to true or input.

core.safecrlf
           If true, makes Git check if converting CRLF is reversible when end-of-line conversion
           is active. Git will verify if a command modifies a file in the work tree either
           directly or indirectly. For example, committing a file followed by checking out the
           same file should yield the original file in the work tree. If this is not the case for
           the current setting of core.autocrlf, Git will reject the file. The variable can be set
           to "warn", in which case Git will only warn about an irreversible conversion but
           continue the operation.

           CRLF conversion bears a slight chance of corrupting data. When it is enabled, Git will
           convert CRLF to LF during commit and LF to CRLF during checkout. A file that contains a
           mixture of LF and CRLF before the commit cannot be recreated by Git. For text files
           this is the right thing to do: it corrects line endings such that we have only LF line
           endings in the repository. But for binary files that are accidentally classified as
           text the conversion can corrupt data.

           If you recognize such corruption early you can easily fix it by setting the conversion
           type explicitly in .gitattributes. Right after committing you still have the original
           file in your work tree and this file is not yet corrupted. You can explicitly tell Git
           that this file is binary and Git will handle the file appropriately.

           Unfortunately, the desired effect of cleaning up text files with mixed line endings and
           the undesired effect of corrupting binary files cannot be distinguished. In both cases
           CRLFs are removed in an irreversible way. For text files this is the right thing to do
           because CRLFs are line endings, while for binary files converting CRLFs corrupts data.

           Note, this safety check does not mean that a checkout will generate a file identical to
           the original file for a different setting of core.eol and core.autocrlf, but only for
           the current one. For example, a text file with LF would be accepted with core.eol=lf
           and could later be checked out with core.eol=crlf, in which case the resulting file
           would contain CRLF, although the original file contained LF. However, in both work
           trees the line endings would be consistent, that is either all LF or all CRLF, but
           never mixed. A file with mixed line endings would be reported by the core.safecrlf
           mechanism.

core.autocrlf
           Setting this variable to "true" is the same as setting the text attribute to "auto" on
           all files and core.eol to "crlf". Set to true if you want to have CRLF line endings in
           your working directory and the repository has LF line endings. This variable can be set
           to input, in which case no output conversion is performed.

Chomenor · 2020-03-31T13:36:04Z

As far as I can tell the UI error traces to this line:

tremulous/src/ui/ui_shared.c

Line 1942 in 38cbdc8

Q_strcat(selected_list_cvar_string, 1024, val);

when Q_strcat is called with a null 'val' parameter. The problem only seems to occur when running a qvm, not an so. It doesn't seem to be filesystem specific, since the error occurs in the original filesystem too with the same pk3s loaded. Perhaps somebody familiar with this particular code can look into the problem.

dGr8LookinSparky · 2020-03-31T13:44:16Z

Thanks @Chomenor :) , I'll have some time later today to look into it more deeply. But from initially browsing that part of the code, maybe with how QVMs work I have to explicitly check val[0] != '\0'?

Chomenor · 2020-03-31T13:51:45Z

I don't know, it would depend on how that code is supposed to work. Personally I don't know what that code even does...

Also, I encountered another unrelated issue while debugging that one: the line

tremulous/src/sdl/sdl_glimp.cpp

Line 199 in 38cbdc8

modes[numModes].w = 0;

was writing an invalid array index and corrupting memory.

A potential fix is to change the line

modes = (SDL_Rect*)SDL_calloc( (size_t)numSDLModes, sizeof( SDL_Rect ) );

to

modes = (SDL_Rect*)SDL_calloc( (size_t)numSDLModes + 1, sizeof( SDL_Rect ) );

dGr8LookinSparky · 2020-03-31T13:58:33Z

I'll look into that too. Regarding the bucket stuff, that's a new thing I implemented that is used to semi-randomly select from multiple music files, which music to play in the main menu on startup. It ensures that all of the music files in a particular directory are played exactly once for each startup of the client, before resetting and starting again. It is meant to introduce some variety in the music in the main menu. The specific music files we are using currently are placeholders from previous trem mods, that we'll replace at some point when new theme music is made. The general bucket system can be used for other things potentially, like for map rotations, and anything else that we might want to have selected in that kind of way.

Chomenor · 2020-03-31T14:02:03Z

Do you know if the Bucket_Select_A_Random_Item function is meant to be able to return a null pointer?

dGr8LookinSparky · 2020-03-31T14:06:26Z

Oh jeeze, I think I found the problem, in src/bg_misc.c line 4007, return isn't used. I don't know how it even works in the .so like that, lol.

dGr8LookinSparky · 2020-03-31T15:15:05Z

It looks like that fixed it, I'll get the repos updated accordingly :)

Chomenor · 2020-03-31T15:20:15Z

Sounds good!

Buom01 · 2020-03-31T15:51:39Z

Hey
I'm coming late in the conversation, but 798ad46 probably closes #111

dGr8LookinSparky · 2020-03-31T15:56:00Z

Ah yes, it looks like it. When you get a chance, @Buom01 could you test and make sure that issue has been fixed?

Buom01 · 2020-03-31T16:22:32Z

@dGr8LookinSparky Working fine now !
Moreover, I have some suggestion for new musics, but it could be fine to ask players opinion.
Main menu additional music suggestion

@Chomenor Nice works, that sound promising !

Chomenor added 6 commits April 12, 2018 17:10

new filesystem: Copy in initial filesystem files

f497335

new filesystem: Initial tremulous porting work

c462edc

new filesystem: Refactor source dir definitions to avoid hardcoded so…

85ba05a

…urce dir count

new filesystem: Support reading from mac app bundle path

cfaf833

new filesystem: Fix FS_FOpenFile null handle ptr return values to be …

2334088

…consistent with original filesystem

new filesystem: Build with new filesystem ifdefs by default

83f9701

new filesystem: Run clang-format on everything in filesystem directory

e352456

wtfbbqhax requested changes May 2, 2018

View reviewed changes

Chomenor added 4 commits May 17, 2018 17:53

new filesystem: Initial base directory and hash precedence support

61c7449

new filesystem: Try to fix compile errors

9cd31b3

new filesystem: Try again to fix the compile error

843e850

new filesystem: Mod dir handling work

5100805

wtfbbqhax requested a review from dGr8LookinSparky May 22, 2018 18:25

Chomenor added 3 commits May 24, 2018 01:52

new filesystem: Improved download/pure list handling

7e34608

new filesystem: Rename 'system paks' to 'default paks'

2a2b09a

new filesystem: Prioritize pk3dirs over pk3s with same name

57d4706

Chomenor added 2 commits March 26, 2020 00:21

Merge branch 'master' of https://github.com/GrangerHub/tremulous

9ae3c6d

new filesystem: Initial support for multi-version pk3 manifests

62ba61a

dGr8LookinSparky added 2 commits March 30, 2020 07:51

renderer: make lightmap not cheat protected.

7b39045

default assets: Update the Trem 1.3 default assets to v1.3.0-alpha.0.…

eeeb57f

…14.11

Merge branch 'master' of https://github.com/GrangerHub/tremulous

89ea9ea

Chomenor force-pushed the master branch from 0af4838 to 89ea9ea Compare March 30, 2020 14:39

tremulous: fix the bucket system for QVMs.

798ad46

dGr8LookinSparky mentioned this pull request Apr 3, 2021

Port over @Chomenor's new file system from trem 1.3 GrangerHub/Stellar-Prey#14

Open

New filesystem #76

Are you sure you want to change the base?

New filesystem #76

Conversation

Chomenor commented Apr 15, 2018

wtfbbqhax commented Apr 16, 2018

wtfbbqhax commented May 1, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chomenor commented May 2, 2018

dGr8LookinSparky commented Jan 1, 2020

dGr8LookinSparky commented Jan 1, 2020

Chomenor commented Jan 1, 2020

dGr8LookinSparky commented Jan 1, 2020

Chomenor commented Jan 1, 2020

dGr8LookinSparky commented Jan 1, 2020

dGr8LookinSparky commented Jan 1, 2020

Chomenor commented Jan 1, 2020

dGr8LookinSparky commented Jan 2, 2020

Chomenor commented Jan 2, 2020

dGr8LookinSparky commented Jan 2, 2020

dGr8LookinSparky commented Mar 6, 2020

dGr8LookinSparky commented Mar 6, 2020

Chomenor commented Mar 7, 2020

dGr8LookinSparky commented Mar 7, 2020

Chomenor commented Mar 29, 2020

dGr8LookinSparky commented Mar 29, 2020

dGr8LookinSparky commented Mar 30, 2020

Chomenor commented Mar 30, 2020

dGr8LookinSparky commented Mar 30, 2020

Chomenor commented Mar 30, 2020

cengique commented Mar 30, 2020 • edited Loading

Chomenor commented Mar 31, 2020 • edited Loading

dGr8LookinSparky commented Mar 31, 2020 • edited Loading

Chomenor commented Mar 31, 2020

dGr8LookinSparky commented Mar 31, 2020 • edited Loading

Chomenor commented Mar 31, 2020

dGr8LookinSparky commented Mar 31, 2020

dGr8LookinSparky commented Mar 31, 2020

Chomenor commented Mar 31, 2020

Buom01 commented Mar 31, 2020

dGr8LookinSparky commented Mar 31, 2020

Buom01 commented Mar 31, 2020

cengique commented Mar 30, 2020 •

edited

Loading

Chomenor commented Mar 31, 2020 •

edited

Loading

dGr8LookinSparky commented Mar 31, 2020 •

edited

Loading

dGr8LookinSparky commented Mar 31, 2020 •

edited

Loading