hbashton / hbashton/DS4Windows

Profile save/load bugs found while mutation-testing the serialization code (shift Extras lost + misapplied; legacy lightbar channels; LastChecked locale)

Open
#46 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C#
Stars
272
Forks
10
Avg merge
13d 5h
Merged PRs (30d)
1

Description

While working through #37 (mutation-testing the settings/profile serialization code), the surviving mutants kept pointing at unasserted blocks — and reading those blocks, plus a systematic pass over the rest of the DTO serialization code, turned up some real bugs rather than just missing tests. Collecting them here so they're tracked in one place.

1. LastChecked date corrupted across locales — fixed in #42

AppSettingsDTO.LastCheckString writes MM/dd/yyyy but reads back with a culture-dependent DateTime.TryParse, so on a non-US locale the date round-trips with month and day swapped. Fix already open: #42.

2. Legacy per-channel lightbar colour: green/blue read the Red element

In ProfileDTO.PostProcessXml, the legacy <Red>/<Green>/<Blue> path parses RedColorString into all three channels (the green/blue blocks weren't edited after copy-paste), so an old-format profile loads with the lightbar set to (red, red, red). Fix PR opening shortly.

3. Shift-modifier Extras are never saved

In ProfileDTO.MapFrom (~line 1979), the shift copy of the extras block recomputes hasExtrasValue from dcs.shiftExtras but is missing the trailing

if (hasExtrasValue) { shiftExtrasSerializer.CustomMapExtras.Add(dcs.control, dcs.shiftExtras); }

that the non-shift sibling has (~1928-1931). shiftExtrasSerializer is therefore never populated, the Count > 0 guard at ~2044 can never pass, and ShiftControl/Extras is never written to the profile XML — even though MapTo reads it back. Any shift-mode extras a user configures (rumble/lightbar/mouse-sensitivity under a shift trigger) silently vanish on the next save; re-saving an older profile that did contain them strips them permanently. (The old pre-DTO writer in ScpUtil.SaveProfileOld has the recompute followed by the append, which is how the block clearly lost its tail in the port.)

4. Shift-modifier Extras load into the non-shift slot

In ProfileDTO.MapTo (line 2637), the ShiftControl.Extras block calls

destination.UpdateDS4CExtra(deviceIndex, pair.Key.ToString(), false, pair.Value);

with the shift flag still false — unedited from the copied Control.Extras block — while every other ShiftControl sub-block passes true (and the old legacy reader used true for this element). Loading a profile that contains ShiftControl/Extras (e.g. written by an older DS4Windows) applies the shift extras to the control's normal slot: they fire without the shift trigger held, clobber any normal extras on the same control, and the actual shift slot stays empty. Combined with bug 3, one load+save cycle makes the loss permanent.

I'll open a fix PR covering 3+4 together (they're the two halves of the same round-trip) with a regression test for each direction.

Minor / latent (no runtime effect in current builds, noting for completeness)
  • AppSettingsDTO.ShouldSerializeCustomLed7String uses MAX_DS4_CONTROLLER_COUNT > 7 where its siblings use >= N — currently equivalent because the constant is 8.
  • AutoProfilesDTO.MapFrom sets Controller4 inside the USING_MAX_CONTROLLERS block although slot 4 is a base slot everywhere else — currently unreachable-false, so no effect.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with ProfileDTO.MapFrom, MapTo, and PostProcessXml, comparing the shift and non-shift serialization paths and the legacy Red/Green/Blue handling. Add regression coverage for shift Extras save/load in both directions and legacy lightbar channel loading; done means shift values stay in the shift slot and legacy colours load distinctly.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
desktop
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.