Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Media file versioning #204

Closed
2 tasks done
TheJJ opened this issue Jan 30, 2015 · 9 comments
Closed
2 tasks done

Media file versioning #204

TheJJ opened this issue Jan 30, 2015 · 9 comments
Labels
area: assets Involved with assets (images, sounds, ...) lang: c++ Done in C++ code lang: python Done in Python code nice new thing ☺ A new feature that was not there before

Comments

@TheJJ
Copy link
Member

TheJJ commented Jan 30, 2015

When changes to the media files are done, the engine does not realize the media files need to be reconverted and strange errors may occur. E.g. #202 changes the alpha values of the sprites and thus requires a reconversion. We can't enforce it currently, this is why some versioning needs to be introduced.

Proposal: introduce some media_version variable which hast to be incremented each time a reconversion needs to be done. This version is stored in the assets/ folder and checked by the engine.

Other suggestions?

  • asset versioning (openage/convert/changelog.py)
  • gamespec format hashing
@TheJJ TheJJ added nice new thing ☺ A new feature that was not there before lang: c++ Done in C++ code lang: python Done in Python code area: assets Involved with assets (images, sounds, ...) labels Jan 30, 2015
@franciscod
Copy link
Contributor

If the change only affects, say, sounds, it should be nice not to reconvert all the stuff, instead only reconvert sounds. We might want to fragment the version based on media type.

@TheJJ
Copy link
Member Author

TheJJ commented Feb 4, 2015

Indeed, so this is closely related to #79 and #188.

@franciscod
Copy link
Contributor

untested idea code:

import pickle

target_media_version = {  # these numbers should be bumped when changing media code
    'graphics': 1,
    'terrain': 1,
    'sounds': 1,
    'gamedata': 1,
    'interface': 1,
}

def get_current_media_version():
    return pickle.load(open('assets/versions.p', 'rb'))

def set_current_media_version(media_version):
    return pickle.dump(media_version, open('assets/versions.p', 'wb'))

def get_mismatching_media_modules():
    mismatching = []
    current_media_version = get_current_media_version()

    for modulename in target_media_version:
        if target_media_version[modulename] != current_media_version[modulename]:
            mismatching.append(modulename)
    else:
        return None

    return mismatching

def update_media_module_version(modulename):
    version = get_current_media_version()
    version[modulename] = target_media_version[modulename]
    set_current_media_version(version)

# after finishing converting a module
update_media_module_version('sounds')

# list of modules to be converted
for module_name in get_mismatching_media_modules():
    please_start_converting_this_one(module_name)

This pickling might be improved when nyan arrives though

@franciscod
Copy link
Contributor

this numeric approach has a caveat:

  • let's say at master we have graphics version 5.
  • some branch A modifies something about graphics, sets their asset version to 6
  • another branch B modifies another graphic thing, also sets version 6
  • A gets merged
  • when merging B, we should really check that the graphics gets bumped to 7 at least

@TheJJ
Copy link
Member Author

TheJJ commented Feb 17, 2015

We should hash the version whereever possible, this is persistent across merges.
For non-hashable versions like graphics, the numeric approach is, as you proved, bad.

The problem we have to solve is that some changes to the algorithms used don't require reconversion, but others do. How do we separate between them automatically (probably we'll hit the halting problem..)?

Anyone any other ideas?

@inakoll
Copy link

inakoll commented Feb 17, 2015

This is how I would do it :

Let say we have a file containing hash (sha1?) of all the converted media files. This hash file is under source control and is merged automagically.

  1. Let say this file is up to date on all branches.
  2. The convert script could read this file compute the hash and only reconvert necessary files.
  3. If the hash is still bad after the reconversion, everything needs to be re converted and the hash file should be updated and committed.
  4. Some trusted people should maintain this hash file up to date each time it is really necessary on master.
  5. Others don't care about media files and let the build system figure it out.
  6. The "version" of the media could simply be the hash of this special file.

@TheJJ
Copy link
Member Author

TheJJ commented Feb 18, 2015

The media version might be different accross people. Especially if they use mods, which just work transparently after conversion. In the future, when we officially support mods, this attempt will also fail and is inconsistend accross people.

@franciscod showed that using simple numbering might lead to problems, but thinking over it, it is a non-issue: If two people increase the number, this will lead to a merge conflict. And the human resolving the merge conflict has to increase the number to the appropriate following number.

-> The simple numeric approach will work i'd say.

The hashing for the gamedata format is done by storing the parsed version of the empiresdat.py struct.

@TheJJ
Copy link
Member Author

TheJJ commented Jul 29, 2015

done in #323, the hashing from #265 is pending.

@TheJJ TheJJ modified the milestones: v0.4.0, v0.3.0 Jul 29, 2015
@TheJJ TheJJ removed this from the v0.4.0 milestone Feb 3, 2016
@TheJJ
Copy link
Member Author

TheJJ commented Oct 7, 2016

Upstream with e59f5c8.

@TheJJ TheJJ closed this as completed Oct 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: assets Involved with assets (images, sounds, ...) lang: c++ Done in C++ code lang: python Done in Python code nice new thing ☺ A new feature that was not there before
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants