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
Make sure the resource system respects the liveupdate.enabled flag #8189
Conversation
@@ -479,6 +487,9 @@ namespace dmLiveUpdate | |||
|
|||
Result StoreResourceAsync(const char* expected_digest, const uint32_t expected_digest_length, const dmResourceArchive::LiveUpdateResource* resource, void (*callback)(bool, void*), void* callback_data) | |||
{ | |||
if (!IsLiveupdateEnabled()) |
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.
Q: I know that we are mixing one-line conditionals with and without curly brackets, but which is the style to use in the Defold code base? I noticed that you removed curly brackets in the code you touched a bit further down?
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.
I think it's depending on context.
In general, I want less "fluff" that makes it harder to read (for me)
I personally think that a scope (curly braces) isn't needed when all we do is return a value.
Assignments may be different.
And often I leave the braces if I for instance have debugged the scope using a printf or similar.
@@ -116,6 +117,7 @@ namespace dmLiveUpdate | |||
dmResource::HFactory m_ResourceFactory; // Resource system factory | |||
dmResourceProvider::HArchive m_LiveupdateArchive; | |||
dmResource::HManifest m_LiveupdateArchiveManifest; | |||
bool m_IsEnabled; |
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.
But we don't set it anywhere and it's false
by default
This fixes an issue where the liveupdate system wasn't fully disabled when the project setting was set to false (
liveupdate.enabled=0
)PR checklist
Example of a well written PR description:
### Technical changes
Technical changes:
Technical notes: