-
Notifications
You must be signed in to change notification settings - Fork 160
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
s2dex text engine changes: transient cloning + built-in render kernel #332
Conversation
@@ -551,7 +558,7 @@ all: $(ROM) | |||
|
|||
clean: | |||
$(RM) -r $(BUILD_DIR_BASE) | |||
make -C src/s2d_engine clean | |||
$(RM) -r src/s2d_engine/build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesnt this throw an error if that directory doesnt exist? either way, can you wrap a ifeq ($(TEXT_ENGINE), s2dex_text_engine)
around this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-r -f
is reproducible so it shouldn't error (If you look at the definition of RM
, it should be rm -f
already
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah okay cool, still would be nice to have this be conditional, but not a blocker
#ifdef S2DEX_TEXT_ENGINE | ||
s2d_init(); | ||
|
||
// place any custom text engine code here if not using deferred prints | ||
|
||
s2d_handle_deferred(); | ||
s2d_stop(); | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you put an ifdef'd include in this file too? we dont want no warnings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
$(info Grabbing S2DEX Text Engine from `https://github.com/someone2639/S2DEX-Text-Engine`......) | ||
D2 != $(shell git clone -b HackerSM64 https://github.com/someone2639/S2DEX-Text-Engine src/s2d_engine) | ||
endif | ||
DUMMY != make -C src/s2d_engine COPY_DIR=$(shell pwd)/$(BUILD_DIR)/lib/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Building the text engine files using this dummy command means 1) they can't be parallelized alongside the rest of the repo and 2) they'll build no matter what target is passed, including clean. The better solution is to make a rule for the archive that the s2d engine builds, just like libgoddard has, and then run make -C src/s2d_engine
in the rule for that archive. That rule can then have the s2d engine folder as a |
dependency, and the s2d engine folder can have a rule to clone the s2d engine repo. This would let you get rid of both DUMMY commands.
i have a better idea for this one |
finally it can be ready