shpaass / yafc-ce

Powerful Factorio calculator/analyser that works with mods
GNU General Public License v3.0
54 stars 20 forks source link

Decide what to do with "CA2101: Specify marshaling for P/Invoke string arguments" #180

Closed shpaass closed 3 months ago

shpaass commented 3 months ago

This low-priority task is a part of an effort to make TODOs less obscure and more tractable.

When dealing with inspections, I suppressed CA2101: Specify marshaling for P/Invoke string arguments because the suggested fix crashed the app at the step of loading mods. The reason behind it was not immediately clear, so I left it for later.

Essentially, we need to decide if there's a need to fix anything at all or if we just keep the inspection suppressed.

DaleStan commented 3 months ago

I can't find any cases where CA2101 is generated, or where CharSet.Ansi appears. That said, it appears the correct solution is to either replace the DllImport attribute with [LibraryImport("library-name", StringMarshalling = StringMarshalling.Utf8)], or to call a different method that expects UTF-16 instead of Ansi/Ascii/Utf8.

shpaass commented 3 months ago

My apologies. I forgot to mention the actual changes that the inspection suggested. I'll post them in the coming days.

shpaass commented 3 months ago

I'm sure that it was in LuaContext.cs, but now VS doesn't seem to suggest this change anymore. Then I'll delete the suppression if no one brings arguments to do otherwise this week.

shpaass commented 3 months ago

https://github.com/shpaass/yafc-ce/commit/4952ea2a82f2c6b6e72e2536187026838ec78466 unmuted this inspection as was announced previously.