Skip to content

[ENHANCEMENT] The BaseFileSystem class#449

Open
NotHyper-474 wants to merge 1 commit into
FunkinCrew:developfrom
NotHyper-474:feat/base-fs-class
Open

[ENHANCEMENT] The BaseFileSystem class#449
NotHyper-474 wants to merge 1 commit into
FunkinCrew:developfrom
NotHyper-474:feat/base-fs-class

Conversation

@NotHyper-474

@NotHyper-474 NotHyper-474 commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

This PR adds a base class for file systems to extend, providing shared implementations of methods.
It is an abstract class, meaning it cannot be instantiated directly; therefore, it can only be extended by other classes.

Since most functions have an implementation provided by this class, it makes it easier to make changes to existing FileSystems, reducing the chances of human error or the need to copy-and-paste code.

@NotHyper-474
NotHyper-474 marked this pull request as draft July 9, 2026 01:38
@EliteMasterEric

Copy link
Copy Markdown
Member

This is a great idea. This wasn't done earlier because the previous iteration of FileSystem had no shared functions, but the new one does, so this change is helpful.

@NotHyper-474

Copy link
Copy Markdown
Collaborator Author

I'm keeping this a draft until I figure out why this is breaking mods with the internal code for Funkin'. Oddly, with the public one it works completely fine.

Meanwhile, I've been thinking the name FileSystemBase would better convey the use for this class.

@realvirtu

Copy link
Copy Markdown
Contributor

How is it breaking Funkin'?

@NotHyper-474

Copy link
Copy Markdown
Collaborator Author

How is it breaking Funkin'?

Mods are recognized by the game, but they aren't being able to add any songs, characters or scripts.

@NotHyper-474 NotHyper-474 changed the title [ENHANCEMENT] The BaseFileSystem class [ENHANCEMENT] The FileSystemBase class Jul 9, 2026
@NotHyper-474
NotHyper-474 force-pushed the feat/base-fs-class branch 3 times, most recently from be590b8 to af39bff Compare July 9, 2026 15:42
@NotHyper-474
NotHyper-474 marked this pull request as ready for review July 9, 2026 15:44
@NotHyper-474

NotHyper-474 commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Fixed it! It turned out to me a very silly oversight on my part.
I've only done some basic testing, but everything seems to work as intended now.

@NotHyper-474
NotHyper-474 force-pushed the feat/base-fs-class branch 2 times, most recently from 7472ca9 to d3cf652 Compare July 12, 2026 23:18

@EliteMasterEric EliteMasterEric left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Aside from the merge conflict present here, I'd recommend renaming the class to BaseFileSystem to match the other class names. Additionally, since every file system implements it anyway, we could probably remove IFileSystem entirely at some point, although that is a breaking change.

@NotHyper-474 NotHyper-474 changed the title [ENHANCEMENT] The FileSystemBase class [ENHANCEMENT] The BaseFileSystem class Jul 16, 2026
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.

3 participants