Skip to content

Make count, isEmpty, first, entities and for-of honour tick filters - #5

Open
srikarsunchu wants to merge 1 commit into
diffusionstudio:mainfrom
srikarsunchu:fix/filtered-reads
Open

srikarsunchu wants to merge 1 commit into
diffusionstudio:mainfrom
srikarsunchu:fix/filtered-reads

Conversation

@srikarsunchu

Copy link
Copy Markdown

Fixes #2.

What

count, isEmpty, first, entities() and for … of on a Changed() / Added() / Removed() query now answer for the filter. The issue names the three scalar accessors; reading EntityIterator showed for … of and entities() were built from the archetype list alone and had the same gap, so all five read paths are covered together.

How — option 1 from the issue, with one property pinned down

The reads must not consume the change window, or if (!q.isEmpty) q.each(…) would cost the run its events. And a read inside a running each() must not swap the filter's bound columns from under it, which routing through the shared RowFilter would.

So each result keeps a second RowFilter over the same terms for reads. RowFilter.peek(ticks, horizon) opens it at the live filter's lastSeen without advancing anything; begin() is unchanged and now shares its body with peek(). The probe walks archetypes and rows in the same back-to-front order each() uses, so first is the entity each() would visit first.

  • Unfiltered queries keep their O(1) answers; a filtered read is the scan each() already pays.
  • Sorted views answer through their own ListWalk, which owns the filter their each() runs with (they used to delegate count/isEmpty to the base, whose filter is a different instance with its own lastSeen). Ordered views already delegate everything to the base, so they follow.
  • One existing test (accessor.test.ts › get is not a write) asserted query.count === 1 on a Changed query as an incidental "still exists" check; it now asserts both facts explicitly (world.query(Position).count === 1, query.isEmpty).
  • README queries table and SPEC §8.3 now say which operation consumes the window.

Tests

Six new cases in tick-filters.test.ts: reads answer for the filter; reading does not consume what each() is about to see (and after the run the window is closed); first follows each() order; Added/Removed; sorted views; unfiltered reads untouched.

npm test: 2348 passed across dev / prod / react / solid / types. tsc --noEmit and prettier clean.

Not in this PR

#3 (a Changed reader ordered before its writer) is a scheduling design choice between the options listed there; nothing here changes begin() semantics.

🤖 Generated with Claude Code

On a Changed/Added/Removed query the scalar reads summed archetype rows and
never consulted the RowFilter, so world.query(Changed(Position)).isEmpty
answered the structural question while each() answered the filtered one.
for-of and entities() had the same gap.

Route them through a second RowFilter over the same terms, opened with
peek() at the live filter's horizon: it accepts exactly the rows the next
each() would visit, in the same order, and never advances lastSeen. That is
the property that matters — `if (!q.isEmpty) q.each(…)` must not cost the
run its events, and a read inside a running each() must not swap the bound
columns from under it, which a shared filter would. Unfiltered queries keep
their O(1) answers; a filtered read is the same scan each() already pays.

Sorted views answer the reads through their own walk, which owns the
filter their each() runs with; ordered views already delegate to the base.
README and SPEC §8.3 say which operations consume the window.

Fixes diffusionstudio#2

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.

count, isEmpty and first ignore Changed/Added/Removed filters

1 participant