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

Cleaning up the CMakeLists.txt #107

Merged
merged 7 commits into from Jan 8, 2020
Merged

Conversation

@tannergooding
Copy link
Contributor

tannergooding commented Jan 8, 2020

This cleans up the CMakeLists.txt so we have distinct src and samples projects and does some minimal cleanup so that resources can be resolved relative to the host executable.

@tannergooding

This comment has been minimized.

Copy link
Contributor Author

tannergooding commented Jan 8, 2020

(I expect I have some cleanup for Unix still)

@@ -1,277 +1,11 @@
cmake_minimum_required(VERSION 3.13)

This comment has been minimized.

Copy link
@tannergooding

tannergooding Jan 8, 2020

Author Contributor

The root project defines the minimum version and central options (such as the CXX_STANDARD)

#include <cstdint>

//stdlib
#include <algorithm>

This comment has been minimized.

Copy link
@tannergooding

tannergooding Jan 8, 2020

Author Contributor

Sorted these alphabetically and removed duplicates

@@ -17,6 +17,12 @@ namespace NovelRT::Utilities {
static inline const std::string CONSOLE_LOG_AUDIO = "Audio";
static inline const std::string CONSOLE_LOG_INPUT = "Input";
static inline const std::string CONSOLE_LOG_WINDOWING = "WindowManager";

static std::filesystem::path getExecutablePath();

This comment has been minimized.

Copy link
@tannergooding

tannergooding Jan 8, 2020

Author Contributor

Small helper function for resolving the path to the host executable

@@ -0,0 +1,16 @@
set(NOVELRT_SAMPLES_HEADERS)

This comment has been minimized.

Copy link
@tannergooding

tannergooding Jan 8, 2020

Author Contributor

The samples project just includes main.cpp, references libNovelRT, and copies over its output.

set(NOVELRT_SAMPLES_HEADERS)
source_group(TREE ${CMAKE_SOURCE_DIR} FILES ${NOVELRT_SAMPLES_HEADERS})

file(GLOB_RECURSE NOVELRT_SAMPLES_SOURCES CONFIGURE_DEPENDS *.cpp)

This comment has been minimized.

Copy link
@tannergooding

tannergooding Jan 8, 2020

Author Contributor

@RubyNova, I know you said you had problems with GLOB; but from what I can tell CONFIGURE_DEPENDS ensures that the configuration is updated if the glob changes (i.e. if you add a new file to the disk).

@@ -42,22 +42,29 @@ std::unique_ptr<NovelRT::Input::BasicInteractionRect> memeInteractionRect;

int main(int argc, char *argv[])
{
std::filesystem::path executableDirPath = NovelRT::Utilities::Misc::getExecutableDirPath();

This comment has been minimized.

Copy link
@tannergooding

tannergooding Jan 8, 2020

Author Contributor

This just uses the C++ 17 APIs to combine paths. Ideally, we would have a centralized asset manager that helps manage and resolve these paths (e.g. #81).

lua_close(L);
auto runner = NovelRT::NovelRunner(0, "NovelRTTest");
auto console = NovelRT::LoggingService(NovelRT::Utilities::Misc::CONSOLE_LOG_APP);

auto novelChanTransform = NovelRT::Transform(NovelRT::Maths::GeoVector<float>(1920 / 2, 1080 / 2), 2, NovelRT::Maths::GeoVector<float>(456, 618));

novelChanRect = runner.getRenderer()->createImageRect(novelChanTransform, 3, "novel-chan.png", NovelRT::Graphics::RGBAConfig(255, 0, 255, 255));
novelChanRect = runner.getRenderer()->createImageRect(novelChanTransform, 3, (imagesDirPath / "novel-chan.png").string(), NovelRT::Graphics::RGBAConfig(255, 0, 255, 255));

This comment has been minimized.

Copy link
@tannergooding

tannergooding Jan 8, 2020

Author Contributor

it would probably be helpful if we took std::filesystem::path where we want a file path. It would avoid the need for consumers of the API to call .string()

//setenv("DISPLAY", "localhost:0", true);
L = luaL_newstate();
luaL_openlibs(L);
lua_register(L, "average", average);
luaL_dofile(L, "avg.lua");
luaL_dofile(L, (scriptsDirPath / "avg.lua").string().c_str());

This comment has been minimized.

Copy link
@tannergooding

tannergooding Jan 8, 2020

Author Contributor

We currently include this script as part of libNovelRT, but it (in particular) should likely be exclusively an asset to NovelRTSamples

@@ -0,0 +1,95 @@
find_package(Freetype REQUIRED)

This comment has been minimized.

Copy link
@tannergooding

tannergooding Jan 8, 2020

Author Contributor

We don't define project or other items as these aren't meant to be built outside the root cmake file


# Docs

if (DOXYGEN_FOUND)

This comment has been minimized.

Copy link
@tannergooding

tannergooding Jan 8, 2020

Author Contributor

I don't think we need doxygen for things like samples, but its trivial to pull this out and share if we wanted (basically just move this stuff to a Doxygen.cmake file and then include(Doxygen.cmake) from any project wanting docs).

file(GLOB_RECURSE NOVELRT_LIB_SCRIPT_RESOURCES CONFIGURE_DEPENDS Resources/Scripts/*.lua)
file(GLOB_RECURSE NOVELRT_LIB_SHADER_RESOURCES CONFIGURE_DEPENDS Resources/Shaders/*.glsl)

add_custom_command(TARGET libNovelRT POST_BUILD

This comment has been minimized.

Copy link
@tannergooding

tannergooding Jan 8, 2020

Author Contributor

Notably, rather than dumping all assets to the root output directory, this mirrors the directory structure (e.g. <out>/Resources/Images/novel-chan.png).

This helps organize the output and isolate assets from eachother.


if ((pathLength < 0) || (pathLength == PATH_MAX))
{
// TODO: Get command line from /proc/self/cmdline

This comment has been minimized.

Copy link
@tannergooding

tannergooding Jan 8, 2020

Author Contributor

I don't know if Unix has an API like CommandLineToArgv() which is what is blocking this bit.

@@ -71,7 +78,7 @@ int main(int argc, char *argv[])
auto vec = playButtonTransform.getPosition();
vec.setX(playButtonTransform.getPosition().getX() - 75);
playAudioTextTransform.setPosition(vec);
playAudioText = runner.getRenderer()->createTextRect(playAudioTextTransform, 1, NovelRT::Graphics::RGBAConfig(0, 0, 0, 255), 36, "Gayathri-Regular.ttf");
playAudioText = runner.getRenderer()->createTextRect(playAudioTextTransform, 1, NovelRT::Graphics::RGBAConfig(0, 0, 0, 255), 36, (fontsDirPath / "Gayathri-Regular.ttf").string());
playAudioText->setText("Play Audio");

This comment has been minimized.

Copy link
@tannergooding

tannergooding Jan 8, 2020

Author Contributor

Not visible here, but further down the audio service only takes in a file name (such as sparta.wav): https://github.com/novelrt/NovelRT/pull/107/files#diff-ef810af08ed542e7c7cf6edddb5bd627R113)

Ideally, we would have the asset manage track the full paths and we'd be able to internally track these via custom ids (ints, strings, or something else). That way, you can have <custom-path>/sparta.wav as the loaded asset, but still access it via sparta.wav

@tannergooding tannergooding force-pushed the tannergooding:cmakelist branch from ec7dffc to 056a2d2 Jan 8, 2020
@tannergooding tannergooding force-pushed the tannergooding:cmakelist branch from 056a2d2 to c813ef8 Jan 8, 2020
@RubyNova

This comment has been minimized.

Copy link
Member

RubyNova commented Jan 8, 2020

I've merged in a fix to this branch for the worldobject active state as discussed in the discord. I'll just quickly verify everything is working but I don't see why it wouldn't, the fixes were minor.

Copy link
Member

RubyNova left a comment

LGTM! Nice work!


void TextRect::setActive(bool value) {
WorldObject::setActive(value);
std::for_each(_letterRects.begin(), _letterRects.end(), [&value](auto& x){x->setActive(value);});

This comment has been minimized.

Copy link
@tannergooding

tannergooding Jan 8, 2020

Author Contributor

Do we need to set each letter as active? Shouldn't the textrect being active/inactive impact the individual letters at a higher point (such that leaving the letters always active is fine)

@RubyNova RubyNova merged commit f312b5d into novelrt:master Jan 8, 2020
6 checks passed
6 checks passed
license/cla Contributor License Agreement is signed.
Details
novelrt.novelrt-pr Build #20200108.11 succeeded
Details
novelrt.novelrt-pr (ubuntu_debug_x64) ubuntu_debug_x64 succeeded
Details
novelrt.novelrt-pr (ubuntu_release_x64) ubuntu_release_x64 succeeded
Details
novelrt.novelrt-pr (windows_debug_x64) windows_debug_x64 succeeded
Details
novelrt.novelrt-pr (windows_release_x64) windows_release_x64 succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.