Each one should provide a message that will show up within LÖVE. Stop
relying on nearby prints to the terminal.
I also found some unnecessary ones.
There is some potential here for performance regressions: the format()
calls will trigger whether or not the assertion fails, and cause
allocations. So far Lua's GC seems good enough to manage the load even
with Moby Dick, even in some situations that caused issues in the past
like undo.
This is a backport of a bugfix in pensieve.love. It's not _technically_
a bug here in lines.love, but it seems worth establishing an
architectural invariant (or rather lack of invariant).
LÖVE's standard event loop performs the following sequence of operations
in a single frame:
* process events
* update
* draw
Ideally any mutations to global state happen during the first two
phases, while drawing includes no mutation.
However, there is a special case: `starty`, the top y coordinate for
each each line in the editor. This is used all over the place, and the
cheapest way to compute it is to simply save it while drawing.
However, draw by definition only updates `starty` for lines that are
drawn on screen. To avoid stale data on lines off screen, say after
scrolling, events often clear `starty` for all lines, leaving it to the
next draw phase to repopulate the right lines.
Sandwiched between the above two "however"s, the update phase needs to
gracefully handle `starty` being nil in the occasional frame right after
an event.
I think I've audited all our uses of `starty`, and this commit fixes the
only place that violates this rule.
This only happens when pressing C-n inside a drawing in normal (not
maximized) mode.
The reason is complicated. First, the baseline:
* When pressing C-o:
run.keychord: starty is not nil
run.keychord: current drawing is nil
Drawing.keychord C-o: doesn't set current drawing
plan_draw (only happens in normal mode): clears starty in all lines
Drawing.update: all good ✓
* When pressing C-n:
run.keychord: starty is not nil
run.keychord: current drawing is nil
Drawing.keychord C-n: sets current drawing <===
plan_draw: clears starty in all lines
Drawing.update: BOOM
Ugh.
When is starty set? Drawing.draw. This is why I can clean it up with
abandon. But:
frame = events -> update -> draw
* plan_draw clears starty while processing events
* Drawing.update blows up in here <===
* Drawing.draw is about to set starty
So one possible interpretation here is that this is an unfortunate
interaction of two special-cases:
* draw should never mutate state, but it's just the most convenient
place to track starty
* keystrokes never set current drawing -- except rare operations like
naming and moving points
So the current prescription: any code called from within update needs to
protect against starty being nil.
Ugh X-(
Earlier the ghost while drawing wouldn't quite match the final shape.
Now the math is identical in draw_pending_shape.
It's a little unfortunate that we have this duplication of formulae.
At least there are no other stray calls of App.mouse_x in
draw_pending_shape.
This was complicated. I had to:
- make a couple of tests pending that I discovere were only
accidentally passing for a while. I'll fix those outside the merge
commit.
- copy over the implementation of links from the lines-and-links fork
scenario:
slowly press down mouse button and drag to draw a line
release mouse button
Before this commit the point would jump just a little bit on release,
and points would go slightly to the left of where I expect.
Yet another thing it's hard to write an automated test for.
Thanks Lion Kimbro for pointing out this issue.
I still have to use floats for start/end angles of arcs. That might be a
sign that I don't have the right serialization yet for them. Or that
that feature needs to go.
I started out with a hazy idea of only using 8-bit ints for coordinates,
but now I'm not sure how committed I am to that constraint. While the
width is always 256 units, it might be nice to create long portrait
drawings at some point, whose height is greater than 256 units.
Bugfix: we can now edit drawings again.
Now that I notice I can leave notes in merge commits, I should also
mention that the merge of commit ecba652582 fixed the problem mentioned
in 0b7dbadfe5.
It's a luxury to be able to fix things upstream. Next time I should try
fixing a change like these downstream, and backport it upstream. That
would match somebody else's experience with the network of forks I've
been proposing for years.
This works but has a drawback: since we just adjust Editor.top,
we render everything from Editor.top to bottom of screen. If we pan a
long distance, Editor.top can become a large negative number and the
editor will spend more and more time rendering things outside screen.
I suppose we'll find out what these no-op draws cost us.
Scenario where I ran into it:
start a stroke inside a drawing
press 'o' to switch to circle mode (but any mode switch will do)
The problem is that Text.textinput unconditionally blows away all .y
values. Sometimes we have to wait for a draw() to fill them back in.
The previous commit was failing inside a test that I can't reproduce
manually. Perhaps it's something about how often draw/update run in
practice. Anyways, it's definitely uncovered a real issue.
This is all unfortunate in several ways
- lots of functions have extra args
- table lookups where we used to have a simple variable
- program initialization is a lot more delicate
Let's see if it was worthwhile. Can we now actually build around the
editor component in forks?