fix(delta): MinimizedMode correctness and alert handler cleanup#164
Open
AlbertoAmadorBelchistim wants to merge 5 commits into
Open
Conversation
7e6282b to
6074555
Compare
Contributor
Author
|
I've included yesterday's changes in this branch to resolve conflicts. |
When a bar changes divergence direction during a real-time recalc (e.g. from bullish to bearish divergence), the previously-set series kept its value because only the active branch was written. This left a "ghost" dot of the previous divergence visible on the last bar. Reset the opposite series to 0 explicitly in both branches so the dots reflect the current divergence state only. Only affects the MinimizedMode path; the non-minimized branch already resets both series correctly.
…imizedMode GetMinWidth used `-_candles[i].Open` to represent the magnitude of a negative-delta bar in MinimizedMode, but `_candles[i]` is an empty Candle in that case (the real data lives in `_downCandles[i]`), so the computed sample width was always 0 for negative bars. This could let the `minWidth > barWidth` guard pass incorrectly and render volume labels that no longer fit the bar. Match the formula OnRender already uses: `_candles[i].Close > 0 ? _candles[i].Close : -_downCandles[i].Close`.
… handlers UpAlert and DownAlert subscribed to PropertyChanged with anonymous lambdas, which cannot be detached. The rest of the indicator follows a symmetric +=/-= pattern in the setters of FilterColor / FilterInt properties; the alert subscriptions broke that symmetry. Promote the lambdas to named methods (OnUpAlertChanged / OnDownAlertChanged) so that, if UpAlert/DownAlert are ever reassigned through the UI the same way DivergenceBarsFilter is, the old handler can be properly unsubscribed. No behavioural change today.
…style
- Drop the `_absorptionThreshold` field; it was declared but never
read — the live value is always taken from `_absorption.Value`.
- Replace the O(n) string-concat loop in GetMinWidth with
`new string('0', maxLength)`.
- Normalize indentation to tabs across the file.
No functional change.
The divergence dot renderer only checked the bottom edge of the price panel region, so dots could be drawn above the top of the panel when the candle's Low/High projected off-screen (for example under strong zoom). Add a symmetric Top check and hoist the region lookup.
6074555 to
c602d0b
Compare
Contributor
Author
|
During smoke testing, I identified an issue where the filter toggle doesn't apply immediately. I’ve confirmed this is a pre-existing upstream behavior and not caused by this PR. I've added it to the notes, as I haven't yet found a viable fix after debugging. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
OnCalculate(MinimizedMode) so that flipping divergence direction on the same bar between two divergent states (without passing through the non-divergentelsebranch in between) no longer leaves a stale dot from the previous direction. Realistic only on the live last bar during real-time recalculation.OnRenderto both the top and bottom of the price panel. The previous guard only checkedRegion.Bottom, so under strong vertical zoom the dot could render aboveRegion.Top.GetMinWidthwithOnRenderfor negative-delta bars in MinimizedMode. The old expression read_candles[i]for the negative branch, but_candles[bar]is reset tonew Candle()in that branch ofOnCalculate, so width was measured as0and the volume-label width gate (minWidth > barWidth) failed to suppress labels that did not fit.PropertyChangedlambdas wired toUpAlert/DownAlertwith named handlers (OnUpAlertChanged/OnDownAlertChanged), matching the+= / -=symmetry already used forDivergenceBarsFilterandAbsorption. No behavioural change._absorptionThresholdfield (and its#pragma warning disable CS0414), replace a concat loop withnew string('0', maxLength)inGetMinWidth, and normalize tabs / spaces.No behavioural changes outside the three bug fixes.
Changes
fix(delta): clear opposite divergence series in MinimizedModeOnCalculate(MinimizedMode branch)fix(delta): clamp divergence dots to price panel top and bottomOnRenderfix(delta): align GetMinWidth with OnRender for negative delta in MinimizedModeGetMinWidthrefactor(delta): replace anonymous PropertyChanged lambdas with named handlersstyle(delta): use string constructor and normalize indentationGetMinWidth+ whole-file whitespaceTest plan
Smoke-tested on a liquid instrument with active order flow on the flavors the indicator targets.
Bug-fix verification
Mode = Candles,MinimizedMode = on,ShowDivergence = on,DivergenceBarsFilter.Enabled = on. On the live last bar, induce ticks that flip divergence direction between two divergent states without going through a non-divergent state (e.g. bullish-candle/bearish-delta → bearish-candle/bullish-delta). After the flip, only one divergence dot is visible on that bar; the previous-direction dot disappears.Mode = Bars,MinimizedMode = on— identical expectation.ShowVolume = on, clusters chart,MinimizedMode = on. On a chart zoomed so the bar is narrower than the rendered label width: a bar with negative delta no longer draws an oversized label that overflows the bar. With wide bars: label is drawn and fits inside the bar (no false suppression).ShowDivergence = on. Apply strong vertical zoom so that an up-divergence bar'scandle.Lowprojects to ayPriceaboveRegion.Top(i.e.GetYByPrice(candle.Low) + 10 < Region.Top): the dot is not drawn outside the panel.yPricebelowRegion.Bottom: dot is not drawn (regression check; this case already worked before).Regressions on adjacent surfaces
MinimizedMode = off—ShowVolume = on, clusters: width gate behaves as before (no regression from theGetMinWidthchange; the non-MinimizedMode branch was untouched).UpAlert.Enabled = on, Value = XandDownAlert.Enabled = on, Value = -Y: each alert fires once per bar on threshold crossing. ChangeUpAlert.Value/DownAlert.Valueat runtime →_lastBarAlert/_lastBarNegativeAlertreset and the alert can re-trigger after the new threshold is crossed (this is the behaviour the lambdas guaranteed; the named handlers must keep it).DivergenceBarsFilter.Enabledtoggled at runtime — colors and divergence-candles visibility update as before (untouched in this PR).Absorption.Enabledtoggled at runtime — absorption dots appear / disappear as before (untouched in this PR).Mode coverage (regression only)
Mode = Histogram,MinimizedMode = on/off— divergence coloring on the histogram via_delta.Colors[bar]works as before.Mode = HighLow,MinimizedMode = on/off— delta histogram (_delta) plus the_diapasonHigh/_diapasonLowwhiskers (max / min delta of the bar) render and colorize as before. WithMinimizedMode = onthe two whiskers collapse onto the positive side because both go throughMath.Abs.Mode = CandlesandMode = Bars,MinimizedMode = off— divergence renders via_divergenceCandles/_divergenceDownCandles(this branch ofOnCalculatewas not touched).Notes
Technical/Delta.csonly.ShowAbsorptionDots,AbsorptionDeltaThreshold,UseAlerts,AlertFilter,UseNegativeAlerts,NegativeAlertFilter) are untouched.style(delta)commit produces a noisy whole-file diff because of tabs / spaces normalization in regions that previously mixed both. Reviewing it on its own commit, or with?w=1/ "Ignore whitespace", makes the actual functional change inGetMinWidth(new string('0', maxLength)) trivial to verify.Out of scope - filter toggle immediate apply
Smoke-testing surfaced that toggling
DivergenceBarsFilter.Enabled/Absorption.Enabled(or changing their values) in the indicator settings does not visually apply until either a new bar opens or the user zooms / scrolls the chart. Ticks on the live bar are not enough on their own. The historical bars do update correctly once any of those events happen,but plain UI interaction with the filter alone is not.
This is pre-existing behaviour, not introduced by this PR, and was already present in the original
OnAbsorptionFilterChanged/OnDivergenceFilterChangedhandlers prior to any change here.A focused investigation on a debug branch ruled out the obvious causes:
PropertyChangedhandler does run andRedrawChart()returns cleanly.OnRenderis invoked immediately afterwards (verified via per-call logging).RecalculateValues()from the handler triggers a fullOnCalculate(0..CurrentBar)pass (verified via per-bar logging), which itself repopulates_divergenceCandles[bar]/_absorptionCandles[bar]inline. The orange divergence candles still do not appear.RecalculateValues()- also has no visible effect.BaseDataSeries<T>.Changedevent from the indicator constructor shows that it does not fire on indexer mutations in this scenario, neither from outsideOnCalculatenor during aRecalculateValues-driven pass. The per-bar invalidation channel documented in the SDK is therefore unreachable from the indicator side.CandleDataSeriesissealed, so exposing the protectedRaiseChanged(int bar)ofBaseDataSeries<T>via a subclass is not an option either.OnRenderoverlay drawing a marker for each bar where the candle series is non-empty shows the marker immediately on toggle. The data is being written; ATAS'sCandleDataSeriespainter is what does not pick up the change._candles.UpCandleColor = valueinside theUpColorsetter - does cause an immediate visual update onthe existing candles. The repaint mechanism that works for property-driven changes on a series invoked from an indicator property setter does not appear to extend to indexer-driven changes applied from a sub-object's
PropertyChangedhandler.DivergenceBarsFilter.Enabledfirst (which populates_divergenceCandles[bar]correctly but does not repaint) and then togglesShowDivergence(an unrelatedpublic bool ShowDivergence { get; set; }auto-property on the indicator that only governs custom dot drawing inOnRender), the orange divergence candles appear immediately.ShowDivergencedoes not touch_divergenceCandlesat all — its toggle simply goes through the indicator-property-setter pipeline, which forces a repaint that reads the already-populated indexer data. This isolates the problem to the repaint pathway, not to data freshness or render correctness.This appears to be a limitation of the public SDK surface rather than something this indicator can fix from the outside. The clean fix belongs at the SDK level - for example, exposing a public per-bar invalidation hook on
BaseDataSeries<T>, or having the indexer setter fire the existingChangedevent so the public channelbecomes usable from indicator code. Flagged here for upstream attention, not folded into this PR.