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

Name OOT samples in sample bank 0 #1695

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

louist103
Copy link
Contributor

Name copied samples from OOT in bank 0.
I couldn't take the exact names because the current tooling doesn't allow spaces and symbols so those were removed.
I also noticed even though I removed the sample rate the rom still matched and the wav files the same.

Fix FileName

FIx

Revert "FIx"

This reverts commit 510dcd8c03242f49d88173357c36fd0cc8911945.

Fix
Copy link
Contributor

@Thar0 Thar0 left a comment

Choose a reason for hiding this comment

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

No thoughts on the names themselves yet, but 2 things

  • The samplerate properties should be restored. The samplerate and basenote that the extractor determines for a sample are usually redundant up to multiples of 2, so these properties exist to tweak the samplerate and basenote if the decision made by the extractor isn't a good one. This hasn't been done yet but probably will in the future to correct any samplerates that are too fast or too slow compared to in-game or other sources.

  • Various formatting changes will be overwritten if these files are ever regenerated by the extractor, specifically

    • file -> File
    • 4 spaces to tabs
    • Spaces at the end of the lines

    Since these formatting changes don't interact well with the tools and are inconsistent with the other xmls, they should be reverted.

@hensldm hensldm added documentation Improvements or additions to documentation Audio Needs-second-approval Second approval Needs-first-approval First approval labels Sep 22, 2024
@hensldm
Copy link
Collaborator

hensldm commented Sep 23, 2024

Missing a newline at the end of the xml which is making Jenkins not happy.

@hensldm
Copy link
Collaborator

hensldm commented Sep 27, 2024

While we still have the separate FileName attribute, I feel like we should still use snake_case for the filenames.

@hensldm hensldm added the Waiting-for-author Author needs fix to conflicts or address reviews label Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Audio documentation Improvements or additions to documentation Needs-first-approval First approval Needs-second-approval Second approval Waiting-for-author Author needs fix to conflicts or address reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants