fix tuplets rounding errors

[guiv42]

see #118

[Makoha]

Hi @guiv42

I see that your pull request is commits to tuxguitar-next, which is a branch I haven't managed to build yet.
At the moment I get a lot of errors which seems to be caused by the fact that packages in TuxGuitar-lib have changed from org.herac.tuxguitar to main.java.org.herac.tuxguitar.

Do you have any wise words how to fix this?

[guiv42]

I see that your pull request is commits to tuxguitar-next

Yes, there's a good reason for that. In tuxguitar-next we changed the .tg file format. It's a good opportunity to store TGBeat.preciseStart in saved files instead of TGBeat.start, to fix the rounding errors (not yet implemented, I was waiting for your review)

At the moment I get a lot of errors which seems to be caused by the fact that packages in TuxGuitar-lib have changed from org.herac.tuxguitar to main.java.org.herac.tuxguitar.

I changed this to introduce automated unit testing. What errors do you get?
I can think of 2 possible problems:

1. unit tests fail if your absolute path to tuxguitar folder contains non-ascii characters. If needed, unit tests can be bypassed by adding -DskipTests to your maven build command (e.g. mvn clean verify -DskipTests)
2. Eclipse cannot switch properly between master and tuxguitar-next branches, because its hidden .project files are excluded from git repo.

What I can suggest:

• clone my "preciseDurationLong" branch in a dedicated folder, different from your usual working folder, to avoid conflicts with .project files. Or you may experience problems when coming back from tuxguitar-next to master... (I had to struggle about this before understanding where the problem was)
• completely re-import everything into an independent Eclipse project. The procedure is detailed in contribute.md

[Makoha]

Got it running, thanks for the tips.

I created a new workspace directory and then cloned your branch.
I am by no means an expert in Eclipse, so sometimes I encounter a steep learning curve.

Back to your pull request.
I notice that when selecting a tuplet that is equivalent to a tuplet with a smaller value.
E.g. 12-tuplets, 6-tuplets and triplets are equivalent. Also 10-tuplets and 5-tuplets are equivalent.
Tuxguitar now seems to select the smallest equivalent tuplet.

E.g. If you select 6-tuplets, only the first note will be a 6-tuplet, the other notes will be triplets.

On a sidenode, when I try to increase the TGDuration.QUARTER_TIME value I now get a test error when building.
Is there a way to bypass the tests?

[guiv42]

Got it running

Good! I'm also an Eclipse beginner, and sometimes I feel I'm learning the hard way...

Thanks for your feedback about 6:4 vs 3:2 tuplets, I'll fix that.

On a sidenode, when I try to increase the TGDuration.QUARTER_TIME value I now get a test error when building.
Is there a way to bypass the tests?

Sure: just append -DskipTests to your maven build command (e.g. mvn clean verify -DskipTests)

But I really think we should not change that 960 value. As I mentioned above, it's pretty much hard-coded in all existing .tg files. All file format readers also use this convention, and it seems risky to me to change them all.
Also, TuxGuitar shall remain capable of opening existing files containing invalid measures (created with older versions). This is why I prefer using 2 different "start" attributes in parallel in TGBeat.

[Makoha]

I accept your concern about changing TGDuration.QUARTER_TIME. I just want to still be able to experiment.
I'll look more into your pull request, but these where just my initial notes.

[guiv42]

OK, thanks, that's clear.
793d388 fixes the sextuplet issue.

About my PR, the general idea is this one: we cannot delete TGBeat.start attribute, and we shall assume it is imprecise, just because of the past. So, add on top of it another preciseStart attribute, without rounding errors.
In the future, the ideal solution would be:

• when parsing an input file, set TGBeat.start attribute for each beat (no choice - compatibility with older versions)
• after full file is read, and whatever the format, update all preciseStart attributes in the song (easy to implement)
• then, never manipulate TGBeat.start

Ideally TGBeat.setStart() method should only be called from file readers. It's a long way to go before reaching this point... until then, we have to consider this new preciseStart may not always be available: whenever it is needed for some time computation, update corresponding values for all beats in a measure.

[guiv42]

TGBeat.preciseStart attribute is now saved to new file format. Warning: this changes the file format in an incompatible manner.
I merged tuxguitar-next branch back here to solve the conflicts associated to test files

[guiv42]

@Makoha : could you have a look at this PR?
Just tell me if you think you don't have time for this.