Skip to content

Conversation

@evandixon
Copy link
Contributor

@evandixon evandixon commented Feb 7, 2024

Fixes avoids #999, which is real annoying.

A better solution would be to update UIUtilities to select the appropriate ConsoleView transforms instead of PCView. I poked around with that a little but concluded I don't know what I'm doing. I figure this is the safest and simplest solution to just fix the map.

Tested using a laptop with a Logitech Gamepad F310 that was able to repro the issue, though I'll put it through some real testing later on a steam deck.

@evandixon
Copy link
Contributor Author

I apologize for all the whitespace changes muddying the PR. I attempted to revert, but Visual Studio doesn't see any changes. LMK if you want me to take another crack at it.

@xADDBx
Copy link
Collaborator

xADDBx commented Feb 7, 2024

Not directly a fix; more like a stop ToyBox from going kaboom when using a controller.
I'll poke @cabarius to take a look and will probably take a look myself too.
Once the review is finished, considering cabarius:main isn't the latest fork anyway, I'll integrate it into the most recent branch (or we'll finally merge #1058, whatever comes first I guess).
After that, I'll probably take the chance to take care of #1015 and maybe #1081 and do a release with this addition?

Copy link
Collaborator

@xADDBx xADDBx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whitespace changes really made the diff annoying; after opening the files manually to take a look, it seems like the try/catch block is the only difference?
I don't have a SteamDeck and haven't really used Wrath with a Controller, so I'll just assume it really does fix the map issue. Other than the Mod.Log vs Mod.Debug it looks okay.

Considering Narria hasn't popped in yet, I'll assume she's still busy. I'll probably Cherry Pick the commit(s) into the currently Maintained Branch to merge the changes (since this PR is for the slightly outdated cabarius:main branch). Otherwise, it looks fine; thanks for the contribution.

As an afterthought: There's probably some Visual Studio settings that mess up the formatting on your side. I'll just reformat the file after merging so you don't need to mind that.

As another afterthought; if you want you can recreate the for the correct branch: xADDBx/ToyBox-RogueTrader@main...evandixon:ToyBox:fix-steam-deck-map and I can just merge there; whatever you prefer

}
}
catch (Exception ex) {
Mod.Log("Exception in LocalMapBaseViewPatch.SetDrawResult: " + ex.ToString());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is, as I assume, an error that will (always) happen in case ConsoleView is active then it's probably preferable to use Mod.Debug to prevent it from appearing in the average user log (to prevent log spam)

@xADDBx
Copy link
Collaborator

xADDBx commented Feb 14, 2024

Merged in xADDBx#20; thanks for the contribution

@xADDBx xADDBx closed this Feb 14, 2024
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