stackotter / delta-client

An open source Minecraft Java Edition client built for speed.

https://deltaclient.app

GNU General Public License v3.0

322 stars 33 forks source link

Add string descriptions for all LocalizedErrors (#23) #140

Closed japanese-goblinn closed 1 year ago

japanese-goblinn commented 1 year ago

Description

Add string descriptions for all LocalizedErrors

Type of change

[ ] New feature (non-breaking change which adds functionality)

Checklist:

[x] My code follows the style guidelines of this project
[x] I have performed a self-review of my own code
[x] I have commented my code, particularly in hard-to-understand areas
[x] I have made corresponding changes to the documentation comments
[x] My changes generate no new warnings

japanese-goblinn commented 1 year ago

Looking good so far! Thanks for taking up this task, it'll really improve the overall user experience of the client when things go wrong. There are also a bunch more error types in the Core package that get displayed to users (if you're using Xcode I think it might display it in the dependencies section even though it's all part of the same repository which is a bit confusing). Would you be able to add descriptions for those too please?

yep, of course. i have a small vacation for a couple of days, after it i'll fix everything and add descriptions to errors you mentioned. thanks for contribution possibility, looking forward to make some more after finishing this one!

stackotter commented 1 year ago

yep, of course. i have a small vacation for a couple of days, after it i'll fix everything and add descriptions to errors you mentioned. thanks for contribution possibility, looking forward to make some more after finishing this one!

Awesome, have fun on your vacation!

japanese-goblinn commented 1 year ago

Hey, @stackotter. I've finished PR. Changes listed below:

Add period at the end of error description, as you suggested above.
Use a multi-line string for errors that looks more convenient to be described in multi-line way. If you think that some of them (or any other) should be rephrased - let me know.
Remove unused errors and code.
Change some code here and there other code to be more swifty and readable
Refactor some somethingId to somethingID. Due to Swift API design guidelines acronyms should be uppercase if not in the beginning of variable name (check more details in Follow case conventions section)

stackotter commented 1 year ago

I think there are around 350 occurrences of me using Id in the codebase, and most of the dependencies that I use have also gone with Id instead of ID, so I think it's best that we stick to just using Id for consistency. I also personally prefer Id over ID.

japanese-goblinn commented 1 year ago

@stackotter I fixed the ID's and also noticed that protobuf is prefering ID instead of Id. I guess that can't be fixed for now, because i don't see any configuration of this.

japanese-goblinn commented 1 year ago

@stackotter here you go

stackotter commented 1 year ago

Just going to wait for the build to succeed before I merge. It hopefully should within 10 minutes or so.

japanese-goblinn commented 1 year ago

@stackotter thanks for the possibility to contribute. Looking forward to create new exiting (or not) PR's 😅

stackotter commented 1 year ago

thanks for the possibility to contribute

No, thank you for contributing! :) It's always good to have another contributor on board