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

RFC: Move filefinder to liblcf #137

Closed
Ghabry opened this issue May 6, 2015 · 6 comments
Closed

RFC: Move filefinder to liblcf #137

Ghabry opened this issue May 6, 2015 · 6 comments

Comments

@Ghabry
Copy link
Member

Ghabry commented May 6, 2015

Having a filefinder when only working with liblcf would be quite useful sometimes.
For LCF2XML I added my own case-insensitive RPG_RT.ini search but for more complex programs targeting liblcf file finding is probably needed...
Your opinion?

There are 2 boost dependencies:

#include <boost/container/flat_map.hpp>
#include <boost/optional.hpp>

Flat-map could be replaced with std::map and boost::optional with maybe... empty string?

Another issue is the reference of Main_Data::project_path and Utils::ToWideString.

RTP stuff could be kept in the Player, is not generally useful.

@fdelapena
Copy link
Contributor

Looks good to me, however there will need to pass some output logging level from liblcf to player (debug, warning, error) from filefinder messages.

Will this feature move/improve encoding detection into liblcf too?

@fdelapena
Copy link
Contributor

RTP stuff could be kept in the Player, not generally useful.

I think it could be useful for Editor project import, specially the RTP table replacement.

Also, moving map tile reading logic would benefit dupe code in Editor and reusable in other potential tools like map rendering or exporting to pictures, but I think this is out of the scope of this RFC.

@fdelapena fdelapena added the RFC label Jul 31, 2015
@Ghabry
Copy link
Member Author

Ghabry commented Mar 26, 2016

@carstene1ns proposed the first LCF Tool that needs filefinding but not LCF parsing (gencache).
In the long term I see the FileFinder as an abstraction layer over any type of directories/files, e.g. in zip archives. Like a more modern version of PhysFS ;)

Potential candidates we already have are:
Directory access: opendir vs. FindFirstFile vs. standalone mode in Android (Reading APK)
File access: fopen vs. _wfopen vs. sdmc_open (3DS fopen lacks utf8) vs. ZIP/APK access
And helper functions for cooperation with other libraries because they all support custom file functions: "ToSDLRWOps", "ToLibPng", "ToLibOGG" and so on.

So maybe it makes sense to not merge the filefinder into liblcf (except stuff that will be only useful in combination with liblcf, like the RTP table) and create another library instead 😱

@Ghabry
Copy link
Member Author

Ghabry commented Jun 1, 2019

Based on current development I don't think that a filefinder is needed anymore in liblcf.

The requirements per application are too different.

The editor also got a file finder now but it is not comparable to the player filefinder at all.

Only has case insensitive search in a folder via Qt and multi extension search. Is very compact code.
Imo complecated RTP lookup isn't needed for it.

All the complexity for caching, platform dependent optimization etc is not needed here.

The tools could use std:: filesystem but this is c++17.

@fdelapena
Copy link
Contributor

fdelapena commented Jun 1, 2019

Having own implementation in Editor already and if we can use c++17 in tools if needed (not a critical component to be portable on exotic platforms), I'm in favor to reject this RFC.

@Ghabry Ghabry added the Invalid label Sep 24, 2020
@Ghabry
Copy link
Member Author

Ghabry commented Sep 24, 2020

imo this isn't really needed anymore, lcf consumes normal i/ostreams and the requirements are too different depending on the app.

@Ghabry Ghabry closed this as completed Sep 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants