Module indexation off by 1 within pattern ?

[jhw]

Hi,

So I created a simple example to test module indexation, specifically within patterns.

I can see that within the modules, indexation starts at zero - Output has index zero by default. Let's say I add a DrumSynth as in the example below - it will have an index of one, as printed out from the example below.

But let's say I now add a simple pattern containing some notes for this module to play - to make it work, the notes seem to require a module index of two, which seems to indicate that note module indexation starts at one rather than zero - is that correct ? If so why ?

Makes it very confusing if you are trying to synchronise creation of new modules and notes to play those modules

[suggest u test the example below]

if __name__=="__main__":
    from rv.api import Project
    proj=Project()
    from rv.modules.drumsynth import DrumSynth
    drum=DrumSynth()
    proj.attach_module(drum)
    print ("module indexes: %s" % [mod.index
                                   for mod in proj.modules]) # module indexation starts at zero
    proj.connect(proj.modules[1],
                 proj.modules[0])
    from rv.pattern import Pattern
    pat=Pattern(lines=8, tracks=1)
    from rv.note import Note
    def notefn(track, i, j):
        if 0==i%4:
            return Note(note=1,
                        module=2) # eh ?? pattern module refs start at two ??
        else:
            return Note()
    pat.set_via_fn(notefn)
    proj.patterns.append(pat)
    from rv.lib.iff import write_chunk    
    with open("tmp/hello_world.sunvox", 'wb') as f:
        for name, value in proj.chunks():
            if not name:
                continue
            write_chunk(f, name, value)

[jhw]

Just as an API comment, it would be much nicer to be able to reference modules via name rather than index. Assuming you give your modules unique names, much nicer to be able to say "connect my_drum_synth to my_filter", because when creating notes you might then be able to say "note.module = my_drum_synth". Otherwise you have to know what module indexes RV is creating behind the scenes, and do the mental gymnastics of tracking them all. In practice, I always create a dict of {name:index}, so RV may as well do it for you.

[matthewryanscott]

The reason is that module number 0 in a pattern note means "no module number was placed in this cell"... basically a null value. If there is an actual module number entered, SunVox stores that internally as the module index + 1.

Agreed that we should be able to assign note.module with an actual module, or None for no module, and have it do the correct index assignment.

My proposal:

• Add Pattern.project and Note.pattern attributes, and a Note.project property, so that we can look up modules given a note's module index.
• Make note.module a property:
  • getter will return the actual module instance, or raise an error if not attached to a project.
  • setter will accept a module instance in the same project
• Add note.module_index attribute which will store None if empty, or the 0-based module index.
  • (to be auto-converted to the file format's 1-based index behind the scenes)

This would mean a breaking change in the API, but I think we can handle that. 😄

I may have some time over the weekend 🤞 to work on this and some other things in this project, so unless you want to take a try at it, I don't mind putting together a PR for this issue.

(An alternate proposal would be to make the existing Module.index 1-based instead of 0-based... but I would prefer to avoid that as we would probably have to touch more parts of the codebase to achieve that, and wouldn't get any nice new API from making that effort.)

[matthewryanscott]

@jhw there are some nice things in Python 3.7 that I'd like to take advantage of, one of which is the new standard library dataclasses package which I'd like to use to replace our current use of the attr package.

The other thing I'd like to do is add type annotations, which are handled more efficiently in Python 3.7 than they are in 3.6.

So I'm thinking about bumping the minimum version of Python to 3.7 while we tackle some of these changes we're working on. Would this have a strong negative impact on what you're doing?

[jhw]

Agreed that we should be able to assign note.module with an actual module, or None for no module, and have it do the correct index assignment.

great

This would mean a breaking change in the API, but I think we can handle that. 😄

sure

OK you work on this one and I'll work on trying to finish the patch decompiler if that's OK

Rgds

On Thu, 28 Feb 2019 at 23:18, Matthew R. Scott notifications@github.com wrote:

The reason is that module number 0 in a pattern note means "no module number was placed in this cell"... basically a null value. If there is an actual module number entered, SunVox stores that internally as the module index + 1.

Agreed that we should be able to assign note.module with an actual module, or None for no module, and have it do the correct index assignment.

My proposal:

• Add Pattern.project and Note.pattern attributes, and a Note.project property, so that we can look up modules given a note's module index.
• Make note.module a property:
  • getter will return the actual module instance, or raise an error if not attached to a project.
  • setter will accept a module instance in the same project
• Add note.module_index attribute which will store None if empty, or the 0-based module index.
  • (to be auto-converted to the file format's 1-based index behind the scenes)

This would mean a breaking change in the API, but I think we can handle that. 😄

I may have some time over the weekend 🤞 to work on this and some other things in this project, so unless you want to take a try at it, I don't mind putting together a PR for this issue.

(An alternate proposal would be to make the existing Module.index 1-based instead of 0-based... but I would prefer to avoid that as we would probably have to touch more parts of the codebase to achieve that, and wouldn't get any nice new API from making that effort.)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/metrasynth/radiant-voices/issues/27#issuecomment-468478524, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAilGs8ilwMfXogaci6kc7N0hm0tkosks5vSGO2gaJpZM4bT1jp .

[jhw]

I would have to upgrade and in particular check that 3.7 is well supported on Ubuntu, but basically fine

On Thu, 28 Feb 2019 at 23:21, Matthew R. Scott notifications@github.com wrote:

@jhw https://github.com/jhw there are some nice things in Python 3.7 that I'd like to take advantage of, one of which is the new standard library dataclasses package which I'd like to use to replace our current use of the attr package.

The other thing I'd like to do is add type annotations, which are handled more efficiently in Python 3.7 than they are in 3.6.

So I'm thinking about bumping the minimum version of Python to 3.7 while we tackle some of these changes we're working on. Would this have a strong negative impact on what you're doing?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/metrasynth/radiant-voices/issues/27#issuecomment-468479457, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAilOEo0rrS63XzfYZWum97BvD31vkGks5vSGSTgaJpZM4bT1jp .

[matthewryanscott]

@jhw I've added a .mod property to notes, that you can use to get/set the module using actual Module instances rather than 0- or 1-based ints.

Let me know what you think.

[jhw]

Sorry for delay in replying, very busy on other stuff. I'll take a look! Thx