Bugs i found recently

Tue Nov 5 06:33:46 UTC 2013

On Tue, Nov 5, 2013 at 2:01 AM, Dirk Hohndel <dirk at hohndel.org> wrote:

> On Sun, 2013-11-03 at 23:15 +0100, Anton Lundin wrote:
> > Hi there.
> >
> > I thought that i should test the software some, so i undertook the task
> > of digitalizing my 30-so dives that i logged in a old notebook and i
> > found some bugs.
>
> Thank you - that's much appreciated! And exactly what I am hoping people
> are doing right now :-)
>
> > Some easy and straight forward ones:
> > * When pressing "ok" after edited a dive, all the open trips in
> >   the dive list gets folded. This is quite annoying, because it moves
> >   the scroll to.
>
> This seems to be caused by us calling DiveListView::reload() from
> MainWindow::refreshDisplay() - doing so loses the 'expanded' state of
> trips.
>
> When playing around with this I found that we also always fell back to
> sorting by column 0 in descending order. I was able to fix that one, but
> got lost trying to figure out how to remember the expanded state for the
> trips.
>
> We have a bit for that in our data structures, but when trying to
> connect to the expanded() signal of the TreeView in order to keep that
> bit in sync with what's displayed... I still have a lot to learn about
> Qt programming, I guess :-/
>
> > * Merge selected dives only merges two of tree selected. Now when i
> >   tested with 5 selected dives it merged the first tree of them.
>
> Urgs. This was a subtle bug. When looping over the dive list with
> for_each_dive we increment 'i' in every step - but if two dives get
> merged, then the indices of all following dives change (we have one
> fewer dive in the list after all).
>
> Playing with that while testing showed that there still are bugs hidden
> in the dive merging code if you merge dives that shouldn't be merged.
> E.g. when the dives are not close together (I think we should just
> refuse to merge those), but there appears to also be an issue with
> getting the tank pressure in the samples correct. If the second dive is
> on a fresh tank (which basically means that most likely you shouldn't
> have merged these two dives, right?) then that isn't correctly reflected
> in the merged dive.
>
> > * Unable to remove pressure information from a cylinder. The old value
> >   comes back.
> > * When rescaling the window and the different pains, the
> >   information-overlay can end up off-screen.
>
> Those two sound very much like something that Tomaz should look into :-)
>
>
Fixed. :)

> > And a couple of harder:
> > * multiple times i noted that the ui just went bad, and refused to
> >   switch between dives, or kept showing the divetrip info instead of the
> >   dive pain. This happened after working with the ui for 10+ minutes.
>
> That's scary.
>
> > * I noted that either via selecting multiple dives or some bug i
> >   accidentally edited the wrong dives, and wrote over other dives. This
> >   might be due to ui messing up the selections, or due to other bugs.
> >   A problem here is that the ui doesn't give you any feedback when you
> >   are actually editing multiple dives.
>
> Even more scary. That's the type of bugs I really want to get rid of
> before a release. I know this sucks, but can you try and create a
> sequence of steps that reproduces this? Ideally with the test dives so
> that the developers can reproduce it? Or you can do this with your dive
> file if you are welling to share that with us.
>
> I stressed Subsurface with my medium sized XML file for a while (I have
> about 260 dives in it, but the file is large and complex as many of
> these dives have multiple dive computers) and haven't managed to get
> things out of sync in a way that corrupted data.
>
> But I agree with you that we need visual feedback when editing multiple
> dives. So I implemented at least that.
>

One thing that can help finding those bugs:
Since we are working with Qt model system, we *must* not select or deselect
dives manually, but we need to ask the model to desselect it. ( this is
also true on the Gtk Version, not qt related, but model view related. )

When one part of the code 'select_dive' and that doesn't update the Model,
the current selection for the user is what's painted on the dive list, not
what's correctly selected via code, so the user edits something thinking
that he's editing the *visual selection* whereas he's really editing the
newly dive created by add dive. :)

I'v found a couple of places that a 'select_dive' or 'unselect_dive' is
being used outside of the model control, and this is bad and can cause this
kind of wrong behavior.

>
> > And one missing feature that we had in the gtk version:
> > * Ability to add gaschange and bookmark events to a dive.
>
> I vaguely remember having started to work at this at some point... but
> clearly nothing ever came of it. Something for the todo list.
>
> > The only part i'm worried about is the fact that the ui messed up after
> > working with it for a while and maybe tricked me in to corrupting data.
>
> Yes, that is my main worry as well - which is why I asked you to try and
> reproduce the issue.
>
> Thanks for the detailed report!
>
> /D
>
> _______________________________________________
> subsurface mailing list
> subsurface at hohndel.org
> http://lists.hohndel.org/cgi-bin/mailman/listinfo/subsurface
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.hohndel.org/pipermail/subsurface/attachments/20131105/970e2741/attachment.html>