milkytracker / MilkyTracker

An FT2 compatible music tracker
http://milkytracker.github.io/
Other
1.72k stars 162 forks source link

Colors break when migrating to latest git versions #29

Closed sagamusix closed 9 years ago

sagamusix commented 9 years ago

Possibly due to commit https://github.com/Deltafire/MilkyTracker/commit/d315a478f7f131087aa8da6e1b30d02116c5c439, the waveform display changes its colour to black on black when using a recent MilkyTracker git version with an existing config file from an older MilkyTracker version. I guess new colours need to be appended at the end of that enum rather than inbetween. If that is indeed the case, it may also be worth checking if increasing the number of colours will break (de)serialization of the config file in any other way, too.

dwhinham commented 9 years ago

I guess when I wrote that, I wanted to group similar/related colour settings together logically, but neglected to realise that this would break things. Do you have an old config file handy for me to test and I'll try to improve this? Thanks :smile:

sagamusix commented 9 years ago

Sure, this is what it looks like right after a fresh 0.90.86 install. http://sagagames.de/stuff/milkytracker.cfg

dwhinham commented 9 years ago

Thankyou. I've got that many copies of work-in-progress git versions all over the place, all "version 0.90.86", that it can be confusing :) I'll get on this now.

Deltafire commented 9 years ago

i wonder if we should do something like using odd numbers for development, and even numbers for release - then you'd be able to check the version number when loading the config (if required).

dwhinham commented 9 years ago

That'd be cool. I have still yet to learn how version numbers are done in the real world. Is there any way we can tack the first 5 SHA-1 Git revision characters onto the end of a debug build's version number? Is that even a thing?

sagamusix commented 9 years ago

What OpenMPT does is using the lowest part of the version info for development versions (e.g. 1.25.01.00 = release, 1.25.01.01 - 1.25.01.99 = dev). Since MilkyTracker has one minor version number less than OpenMPT, this would only work by making the next official version 0.91 (which is probably not too bad). It would also mean that you'd have to reach a sensible 1.0 status within the next 10 versions. :) Either way it would probably not be a bad idea to do it like this - after all, what does the 90 and the 86 mean, anyway?

sagamusix commented 9 years ago

@dwhinham if the VCS is set up correctly, you can extract the commit number in a pre-build step and then put it into a version string of some sort. It wouldn't fit into the XM header, but it could at least be displayed in the program itself (e.g. "0.90.86 commit d315a47").

Deltafire commented 9 years ago

I did look at using the git commit hash (I think I did this with Hively and/or Klystracker), but the problem is that it's a pre-build step as sagamusix said - which means it'd need to be implemented at least 3 times (sdl autoconf scripts, xcode, vs12, then all the older build systems). I also wondered whether a git hook could populate a file somewhere, perhaps there's a github plugin for this..

I like sagamusix's idea.

dwhinham commented 9 years ago

:+1: on @sagamusix's suggestion re: version number scheme; I always did wonder where the 90.86 came from. Yeah, I understand the hassle with all the build systems. If we can find a simple-ish way with git hooks, then great :smiley:

dwhinham commented 9 years ago

Finally got round to checking this.

The problem wasn't to do with the order of the enums - it turns out I was sensible enough to not disrupt the ordering of the stuff that actually deals with the (de)serialisation, so the new colours are indeed at the end of the palette string in the config file.

The problem is that an old config just stores (0, 0, 0) RGB values in the unused space in the palette string, so when loaded into the current git version of MT, the new colours just get zero (black) values.

I've added a simple version check to give these colours their default values if the config is being upgraded. There's a caveat though - dealing with the stored colour presets (i.e. not the active one) is more difficult, as MT seems to have arbitrary colours in the presets' spare spaces where I'm now storing the new colours (instead of just black). The result is some strangely coloured scrollbars etc. The workaround is to just fix your presets if you've customised them, or export your active colour scheme, hit restore to set the presets to their defaults, and re-import your active colour scheme. In the commit which added the new colours, I actually gave each default preset new matching scrollbar colours etc, so hitting restore will make them current.

I'll probably do another commit that zeroes these spare colours to avoid this in future. In the meantime, care to test 57f8290? :smiley:

sagamusix commented 9 years ago

Seems to work fine. :)