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

Access standard application data and asset directories for all supported platforms #72

Merged
merged 8 commits into from
Mar 26, 2025

Conversation

alistair-baxter-kdab
Copy link
Contributor

@alistair-baxter-kdab alistair-baxter-kdab commented Mar 17, 2025

Plus a series of other additions to the likes of KDUtils::Dir requires to support these, and test them.
Added proper file and directory support for Android to allow reading, and in the case of non-assets writing.
Added a filesystem example, including an Android project, to road-test filesystem functions within a running KDGui application.

The various commits in this PR are intended to be merged by rebase-and-FF rather than squash.

Change-Id: Id7d92faf1f100509fcecc93e4dd0ebce394a8997
@alistair-baxter-kdab alistair-baxter-kdab force-pushed the appdata_assets_dirs branch 17 times, most recently from d0f6755 to ce3bb56 Compare March 20, 2025 10:01
Copy link
Contributor

@seanharmer seanharmer left a comment

Choose a reason for hiding this comment

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

Do we need to use signals for the touch events? Can't we subclass in KDGui like we do for xcb or wayland? I'm not familiar enough with Android to know if there are some additional restrictions.

@alistair-baxter-kdab alistair-baxter-kdab force-pushed the appdata_assets_dirs branch 2 times, most recently from 2691b01 to bacc966 Compare March 20, 2025 12:46
Gets the parent directory, and determines whether it has one.

hasParent
@alistair-baxter-kdab
Copy link
Contributor Author

Do we need to use signals for the touch events? Can't we subclass in KDGui like we do for xcb or wayland? I'm not familiar enough with Android to know if there are some additional restrictions.

It turns out that the need to do that refactoring was a misconception on my part, so I have just dropped it.

@alistair-baxter-kdab
Copy link
Contributor Author

Whoops, apparently I don't understand what the GitHub UI buttons do.

@alistair-baxter-kdab alistair-baxter-kdab force-pushed the appdata_assets_dirs branch 5 times, most recently from a53f20f to 70724cd Compare March 24, 2025 19:10
@seanharmer
Copy link
Contributor

Looks better to me now. Happy once the linter is silenced.

Change-Id: I37dd876d65110c3d90c966c3f6fff66cf9f94618
Change-Id: Idf3d956431c8ba620e011aba94972521cb329326
To the core application class, add the ability to find Directories
necessary for storing application data files and assets. Calculate
the locations of these based on application metadata, as need be.
Include the ability to switch between local/internal and roaming/
external appdata folders on Android and Windows.

On Android, assets must be dealt with by different API from regular
files, so add a common storage type element to files and directories.
Dealing with this on Android will be implemented in a subsequent
commit.

Change-Id: Idb63275e1991d1aa813f47f3a6431455f99666f6
Provide ways of getting files and subdirectories that preserve
directory storage type.
Remove Android #ifdefs from base file type.
Use an alternative file implementation on Android, that uses
appropriate functionality based on storage type.
Implement Dir::exists for Android asset directories.

Change-Id: Ia987a7383440d597635e32b9ea81a89aa119710d
Because the unit tests do not runon Android, create an example where we
can test Android's filesystem behaviour, in comparison to the same tests
run on other platforms.

Include example assets, in the form of a simple text file.

Change-Id: I7f700ce39115fb82c021b89c2a8712e2949b1797
@MiKom
Copy link
Member

MiKom commented Mar 25, 2025

I took a look at the assets/ folder situation. I think we can simplify it and contain the folder to the example that it's related to.

How about:

  1. move the assets/ folder to examples/filesystem
  2. Remove all CMakeListst.txt files in the examples/filesystem/assets
  3. Add the following to examples/filesystem/CMakeLists.txt (under add_executable):
add_custom_command(
  TARGET ${PROJECT_NAME}
  POST_BUILD
  COMMAND ${CMAKE_COMMAND} -E copy_directory ${CMAKE_CURRENT_SOURCE_DIR}/assets $<TARGET_FILE_DIR:${PROJECT_NAME}>/../assets
)

With this approach we also won't have all the CMake cruft that's getting generated in the build folder in assets/ right now:

$ ls assets/
CMakeFiles  CMakeLists.txt  CTestTestfile.cmake  cmake_install.cmake  text
mikom@dell-laptop:~/kdab/rnd/kdutils/code/KDUtils/build/Debug$ ls -R assets/
assets/:
CMakeFiles  CMakeLists.txt  CTestTestfile.cmake  cmake_install.cmake  text

assets/CMakeFiles:

assets/text:
CMakeFiles  CMakeLists.txt  CTestTestfile.cmake  asset_text.txt  cmake_install.cmake

assets/text/CMakeFiles:

This also works with multi-config CMake generators like VS or Ninja Multi-Config, unlike the current solution.

@alistair-baxter-kdab alistair-baxter-kdab merged commit baaacbb into main Mar 26, 2025
64 checks passed
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