Skip to content
New issue

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

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

Already on GitHub? # to your account

[STCC-155-224] rename media files #155

Conversation

oiduh
Copy link
Contributor

@oiduh oiduh commented Mar 7, 2020

Initially all costumes and sounds were named using the following information from the scratch project file:

  • md5-hash of the file = 32 character long id
  • costume name or sound name

Since the same images result in the same md5-hash and different objects can have a costume with the same name, it was possible that some files had the same name, which resulted in overwriting of existing files -> not all costumes and sounds had their own resource file. This was a problem for catrobat, because deleting e.g. a costume of an object could also have an impact on other objects (costume deleted as well).

The new proposed solution is to name the files as following:

  • use md5-hash of the resource file and extended with an id ("_#0")
  • if a md5-hash was already used, extend it with an increased id

These names are used until conversion of all unconverted files is finished, then the names of the catrobat files are replaced with something like "img_#0.png" and "snd_#0.wav" to keep it short and simple.


It was also considered to name the files after object and costume/sound name, which resulted in the following problems:

  • illegal characters in object name or image/sound name could cause problems for various operating systems -> replacing them with different characters could result in a filename collision (initial problem)
  • scratch allows very long names for objects and costumes/sounds -> using them for filenames results in conversion failure (filename too long); slicing the names could again result in filename collisions

One last possible solution to name the files after object + costume/sound would be to cut the names to a certain length and add an id at the end, if collisions occur -> debatable how useful this naming convention is.

def _generate_mouse_filename():
mouse_path = _mouse_image_path()
return common.md5_hash(mouse_path) + "_" + MOUSE_SPRITE_FILENAME
def _get_mouse_filename():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The call to this function in converter.py line 688 was not adapted to the new name so there is an error when converting a project with mouse cursor. This should be fixed.

for info in catrobat.media_objects_in(self.catrobat_program):
basename, _ = os.path.splitext(info.fileName)

# ignore these files, already correctly provided by the converter
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a thought: key sprites are now the only ones left being saved with a hash in their name. If those are not necessary, it would be cleaner to remove those hashes too. This could maybe be done in _key_filename_for in converter.py.

@oiduh oiduh force-pushed the STCC-155-224_rename_files_and_create_copy_for_each_costume_and_sound branch from 5a49c53 to 92cb95e Compare April 2, 2020 09:50
@oiduh oiduh force-pushed the STCC-155-224_rename_files_and_create_copy_for_each_costume_and_sound branch from 92cb95e to c8276c3 Compare April 2, 2020 10:09
@oiduh
Copy link
Contributor Author

oiduh commented Apr 2, 2020

All requested changes were made. Regarding the second comment: i changed how the filenames for mouse cursor and keys are created initially, so they already have a name without hash.

Copy link
Contributor

@sandra0521 sandra0521 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code is reviewed, tested sprites, sounds, keys and mouse. Everything fine.

@AntiDog AntiDog merged commit 04d5656 into Catrobat:develop Apr 8, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants