MobArena icon indicating copy to clipboard operation
MobArena copied to clipboard

Fix potential null indirections

Open tadhunt opened this issue 1 year ago • 3 comments

Summary

  • This is a…
    • [X] Bug fix
    • [ ] Feature addition
    • [ ] Documentation
    • [ ] Refactoring
    • [ ] Minor / simple change (like a typo)
    • [ ] Other
  • Describe this change in 1-2 sentences:

Problem

Fix code paths that might result in null indirections

  • GitHub issue (optional):

Solution

Action

tadhunt avatar Sep 19 '23 14:09 tadhunt

Regarding refactoring, I can't commit to that as I don't really understand this code and am worried about breaking it in some non-obvious way.

Re: requests above, I didn't see anything directly actionable from your notes. Did you have something specific I should change?

Maybe we just add a comment to the changes in this PR that notes this needs a major refactoring?

tadhunt avatar Oct 21 '23 18:10 tadhunt

Regarding refactoring, I can't commit to that as I don't really understand this code and am worried about breaking it in some non-obvious way.

For sure. Unless you have an irrational desire to refactor moldy spaghetti code that takes several passes just to kinda understand the overall idea of, this isn't something I'd realistically expect anyone to do, so no worries 😛

Re: requests above, I didn't see anything directly actionable from your notes. Did you have something specific I should change?

Maybe we just add a comment to the changes in this PR that notes this needs a major refactoring?

I think the most consistent thing to do here would be to close this PR, but move the change with the default branch in WaveParser over to the commit in #773 that already deals with fixing switch statements, since it fits really nicely in there. I'm more than happy to assist with wrangling git 😃

garbagemule avatar Oct 21 '23 23:10 garbagemule

SGTM!

tadhunt avatar Oct 22 '23 03:10 tadhunt