Skip to content

Conversation

KazWolfe
Copy link
Member

@KazWolfe KazWolfe commented Sep 7, 2025

Deprecates/tags a few ClientStructs types in Dalamud's public API.

List of detected cases:

Dalamud.Game.Addon.Lifecycle.AddonArgTypes.AddonRefreshArgs.AtkValueSpan.get -> System.Span<FFXIVClientStructs.FFXIV.Component.GUI.AtkValue>
Dalamud.Game.Addon.Lifecycle.AddonArgTypes.AddonSetupArgs.AtkValueSpan.get -> System.Span<FFXIVClientStructs.FFXIV.Component.GUI.AtkValue>
Dalamud.Game.ClientState.JobGauge.Types.SMNGauge.AetherFlags.get -> FFXIVClientStructs.FFXIV.Client.Game.Gauge.AetherFlags
static Dalamud.Interface.Components.ImGuiComponents.HelpMarker(string! helpText, Dalamud.Interface.FontAwesomeIcon icon, FFXIVClientStructs.FFXIV.Common.Math.Vector4? color = null) -> void
static Dalamud.Memory.MemoryHelper.ReadSeString(FFXIVClientStructs.FFXIV.Client.System.String.Utf8String* utf8String) -> Dalamud.Game.Text.SeStringHandling.SeString!
static Dalamud.Memory.MemoryHelper.ReadSeString(FFXIVClientStructs.FFXIV.Client.System.String.Utf8String* utf8String, out Dalamud.Game.Text.SeStringHandling.SeString! value) -> void
static Dalamud.Utility.Numerics.VectorExtensions.ToByteColor(this System.Numerics.Vector4 v) -> FFXIVClientStructs.FFXIV.Client.Graphics.ByteColor

The MemoryHelper and VectorExtensions variants are a non-issue IMO, as they have well-documented alternatives and can be handled in other ways. I wouldn't be opposed to stripping out ToByteColor though, just to avoid confusion with custom CS.

- Remove AtkValueSpan from addon args
- Don't expose SmnGauge enum
- Replace HelpMarker vector4
@KazWolfe KazWolfe requested a review from a team as a code owner September 7, 2025 06:54
@Haselnussbomber
Copy link
Contributor

I actually did run into issues on CStringPointer before, where I had copies of the extensions functions, but still used the ones from Dalamud. I know, third party plugin is not of your concern, but if it can happen to me, it can happen to others as well. We probably shouldn’t make exceptions here. The pitfall is that this explodes in runtime and is not caught on compile.
I don’t know how you did this, but it might be worth adding something like an analyzer to Dalamud.NET.Sdk, whenever the CS reference gets disabled, so we don’t run into this?!

@KazWolfe
Copy link
Member Author

KazWolfe commented Sep 7, 2025

I don’t know how you did this, but it might be worth adding something like an analyzer to Dalamud.NET.Sdk, whenever the CS reference gets disabled, so we don’t run into this?!

I set up the PublicAPIAnalyzer and generated the TXT file, just to see what the damage is. I'd support a more robust way of doing this, but I'm not aware of any type of "banned symbol" analyzer that can only apply to public APIs and I really don't want to make one. (Though, if we are ok making our own analyzer, I'd love an [Obsolete] tag that only affects external usage).

CStringPointer is a bit annoying, because I agree that it's suboptimal and we should probably make a single rule. The downside, though, is that we're going to be removing what's (ideally) a useful function for 80% of users.

Would strong-naming ClientStructs make a difference here? Edit: no:

For .NET Core and .NET 5+, strong-named assemblies do not provide material benefits. The runtime never validates the strong-name signature, nor does it use the strong-name for assembly binding.

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