Open ckolumbus opened 2 months ago
Hi ckolumbus, thank you so much for opening this issue, it's such a great job you've done here ! I've opened a PR to discuss directly about the changes in code you've done. I don't have much time lately, but i'll try to make a review as soon as I can
At first glances these changes seem to be good and seem to fit into the existing logic of the code though :)
Feature
Finger
elementI tried to make an implementation for displaying 'barre' elements and would like to discuss the feature interface but also the implementation approach here.
It would be great if someone can provide feedback on the style of the API and the current implementation. I'd really like to improve this in a way so that it can be merged in this repo!
This could address requests from #7, and to some extend #19.
The spike implementation can be found in my fork.
The rational for the style of implementation is, that the current element (e.g. existing chord fingerings) do not allow for multiple elements on one string. A more flexible modeling of string/fret and multi-string/fret definitions was needed IMHO. But hey, any feedback is welcome ππ½ !
Impl Description
I've added a new element 'finger' which represents a finger id (just a text), a fret number and one string or a range of strings. This is drawn in the normal black rounded rectangle format.
Two new types are used for this (maybe this can be improved, but seemed sufficient for this spike):
The
Fretboard
class has been extended with the following three functionsadd_fingerpos
: add one single finger elementadd_fingerset
: add a set of fingers, e.g. defining a chord fingeringadd_notes_from_fingerset
: take the fingerset and add notes from it, it only add notes that would really sound, not the ones "shadowed" by other fingers on the same string (e.g. G major with full bar on fret 3)During the implementation I found some issues with the implementation of
add_note
andadd_single_note_from_index
where the handling of horizontal and vertical layouts was not working correctly. I'll add an other issue for this but implemented a quick workaround to fix the APIs and use it for the finger implementation.Examples
Define a finger set for
G
on fret 3Display
G
finger setResult
Display
G
finger set with NotesResult
Add Fingerset to scale display
Result
Add single Barre finger position to scale
Result
Feature
Text
elementThe branch above also contains an implementation of a general Text element, which could be used to address #30