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

MiniMap System. #2362

Closed
wants to merge 1 commit into from
Closed

Conversation

dhmello
Copy link
Contributor

@dhmello dhmello commented Aug 1, 2024

Rebuild from @cheshire plugin

@dhmello
Copy link
Contributor Author

dhmello commented Aug 1, 2024

PS: I don't know how to include the assets with this PR. Please help me.

@WeylonSantana
Copy link
Contributor

I do not approve this pull request

I'm refactoring the code to make the code more concise (maybe), renaming variables, cleaning the code and leaving everything without warnings in the IDE and cleaner

You are bringing old and non-functional code (yes, it is functional, but it is implemented incorrectly)

I know this because I already made a minimap system using all the code you sent and I noticed several flaws in how it works
I ended up having to redo this entire minimap system from 0, and all this plugin code just served as inspiration

an example I can say is, your MinimapWindow file has 629 lines
mine minimap window file has 367 lines, and I admit it could be less with the patterns I'm doing recently at the intersect
so there is something unusual about the implementation of this minimap that I can make it all work correctly with practically half the number of lines

Thank you for your intention, but sending a pr could help us more when trying to maintain our standard so that other developers like you in the future can have more references and not look at two files in the same "interface" folder and get confused.

There isn't one weeeeeeellllll standard established yet, but we are trying to building it, there is already one for windows created on the client side and your MinimapWindow doesn't meet any requirements, which you can see in the latest modifications made to the last windows I modified.

other points that make this pull request invalid are that you don't give any description (I know it's a mini system map, but you could have made your own modifications), we need a video showing it working, what changes were made and so on.

I'm going to close this pull request as invalid, but know that we are open to having a minimap here at the base (I know aru and I are), I just need your code to be much more concise and not a copy and paste of a code years ago

If you have any questions or maybe a follow-up on how to do all this, feel free to send me a private message or use the official discord support channel in the "programming support" area, it would be cool to have another developer collaborating with us, so I can answer your questions about how we are doing things now

@dhmello dhmello deleted the MiniMap-Rebuild branch August 1, 2024 12:14
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