-
-
Notifications
You must be signed in to change notification settings - Fork 488
Add getResourceFiles
#3603
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
Closed
TracerDS
wants to merge
20
commits into
multitheftauto:master
from
TracerDS:230724_Add_getLoadedFiles
Closed
Add getResourceFiles
#3603
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
4f3acf9
Add `getLoadedFiles`
TracerDS da7617f
Updated CLuaResourceDefs.cpp
TracerDS adde333
Merge branch 'master' into 230724_Add_getLoadedFiles
TracerDS 8289dcb
Updated code
TracerDS 59052f4
Merge branch 'master' into 230724_Add_getLoadedFiles
TracerDS 17e0c76
Fix MacOS/Linux builds
TracerDS 452dde7
Updated code according to reviews
TracerDS 5e3d05a
Merge branch 'master' into 230724_Add_getLoadedFiles
TracerDS 6f6a8b3
Update CResourceFile.h
TracerDS 6127d68
Merge branch 'master' into 230724_Add_getLoadedFiles
TracerDS 7b78449
Merge branch 'master' into 230724_Add_getLoadedFiles
TracerDS eb831e0
Merge branch 'master' into 230724_Add_getLoadedFiles
TracerDS 543e6cd
Update CLuaResourceDefs.cpp
TracerDS df5275e
Update CLuaResourceDefs.cpp
TracerDS 0d5e2e9
Merge branch 'master' into 230724_Add_getLoadedFiles
TracerDS 859f7f4
Merge branch 'master' into 230724_Add_getLoadedFiles
TracerDS e794557
Merge branch 'master' into 230724_Add_getLoadedFiles
TracerDS 979d2ae
Merge branch 'master' into 230724_Add_getLoadedFiles
TracerDS 04a49af
Merge branch 'master' into 230724_Add_getLoadedFiles
TracerDS a9c3768
Merge branch 'master' into 230724_Add_getLoadedFiles
TracerDS File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Previous varian was more readable. Why did you change it?
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.
previous variant was basically the same.
This leaves room for more conditions to come.
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.
There is no 'early continue' rule. You obfuscate the code with overusing 'continue' in loops. And you don't really need to use early return for small code blocks.
You did the code less readable and maintainable.
Is fine condition, that describes the function logic.
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.
There is early-return which is basically the same but whatever.
I'd rather go with continue rather than indentation
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.
Your
continue
creates indentation too. Please, keep it simpleNot really. Multiple 'continue' conditions can make a loop harder for reading.
There is no goal to use minimal indentation. The code should primarily describe the logic.
You're overusing 'early return'
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.
No one was interested in this convo for 2 weeks, give it a break.
No contributors were opposed against this so Im leaving it as it is.
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 also see the early return overuse here.
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.
And nobody confirm your position. Don't hide requested changes. Stale conversations should be dismissed by another members.
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.
Im following the guidelines is'all.
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.
you interpreter these guidelines very differently