update to use openrs2 and fix some bugs I found#205
update to use openrs2 and fix some bugs I found#205ARTARNA wants to merge 12 commits intoConnorDY:mainfrom
Conversation
|
Mostly some questions in review. Nothing super problematic jumps out though there are a few things I find confusing. The biggest question is why the RuneStats support is being dropped entirely -- kinda big for a PR with no description! Could you expand on the bugs you're fixing? I can see some amount of error handling being added in some places, but it's not clear what this does in the grand scheme of things. |
| }.start() | ||
| } | ||
|
|
||
| private fun downloadCache( |
There was a problem hiding this comment.
this is getting fairly hefty; might be nicer if it had its own class at this point. also it's been a while since I touched RS2 stuff but is there a reason to remove runestats support entirely? are they cancelled? lol
Texture loading has been fixed for newer caches -- no longer need to work around it by skipping the textures
+ Factor out some repeated code + Use consistent user agent + Avoid URL.openConnection
|
I've resolved some of the easier comments I have on this myself (with apologies: it's on the main branch on your fork, so I've had to push up to there!), now what remains is:
Bigger maybes:
I've put in what I have time to put in on this, I'll maybe come back to it when I'm more free and then put some time into some refactoring, or reinstating RuneStats support alongside OpenRs2. Otherwise please discuss in the meantime if you're able! |
- Exceptions are still possible due to a TOCTOU race, so just handle the exception either way. - We get marginally less overhead on the happy path this way (though this is unlikely to mean anything in real life)
2 participants
No description provided.