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

feat: enter market #4842

Closed
wants to merge 2 commits into from
Closed

Conversation

omarcopires
Copy link
Contributor

@omarcopires omarcopires commented Nov 19, 2024

Pull Request Prelude

Changes Proposed

I refactored the action loading logic to improve readability and performance. I created an auxiliary function to simplify ID parsing, avoiding code duplication. I also optimized how attributes like allowfaruse, blockwalls, and checkfloor are configured by using a generic assignment function. Additionally, I improved error logs by including more useful information, such as the action's name when it fails. This makes the code cleaner and easier to maintain.

@gesior
Copy link
Contributor

gesior commented Nov 22, 2024

@omarcopires
It's hard to tell what you changed in data/scripts/actions/actions_xml.lua, because you have changed indent from 4 spaces to TAB character.
You should keep '4 spaces' formatting in that file. Can you replace all TABs to 4 spaces in that file and push commit?
It will be easier to review changes than.

I know that TFS Lua indent is inconsistent, but you should address that issue in other PR (replace all 4 spaces to TAB or all TAB to 4 spaces). I don't even want to know who approved code with a different code indentation. 😞

EDIT:
I also know about your commit to fix Lua/XML indent 5efc56d
but something went wrong and there are new commits that contain 4 spaces intend, A LOT of them:
image
1777 lines of Lua are 'new line + 4 spaces'

Maybe some QC GitHub worker should block PRs with invalid Lua/XML indent?

// CLion IDE knows that files can be formatted different in given project and detects 2 spaces/4 spaces/TAB formatting per file and apply that format, when you press TAB or auto-format code.

@omarcopires
Copy link
Contributor Author

@omarcopires It's hard to tell what you changed in , because you have changed indent from 4 spaces to TAB character. You should keep '4 spaces' formatting in that file. Can you replace all TABs to 4 spaces in that file and push commit? It will be easier to review changes than.data/scripts/actions/actions_xml.lua

I know that TFS Lua indent is inconsistent, but you should address that issue in other PR (replace all 4 spaces to TAB or all TAB to 4 spaces). I don't even want to know who approved code with a different code indentation. 😞

EDIT: I also know about your commit to fix Lua/XML indent 5efc56d but something went wrong and there are new commits that contain 4 spaces intend, A LOT of them: imagem 1777 lines of Lua are 'new line + 4 spaces'

Maybe some QC GitHub worker should block PRs with invalid Lua/XML indent?

// CLion IDE knows that files can be formatted different in given project and detects 2 spaces/4 spaces/TAB formatting per file and apply that format, when you press TAB or auto-format code.

92d6ca9
deabf58

@omarcopires omarcopires deleted the enter-market branch November 22, 2024 20:01
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.

2 participants