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

Fix nulls #16

Merged
merged 5 commits into from
Jul 23, 2020
Merged

Fix nulls #16

merged 5 commits into from
Jul 23, 2020

Conversation

leif81
Copy link
Member

@leif81 leif81 commented Jun 3, 2020

For #14

@leif81 leif81 self-assigned this Jun 3, 2020
@treybgreen
Copy link

See Comment, I do not believe that we should just be reading a single byte, but an entire IffData repeating element, and there are 31 null() calls that need to be fixed.

@leif81
Copy link
Member Author

leif81 commented Jun 4, 2020

Thanks @treybgreen you've given me reason to pause and check the spec.

If I'm reading it right, it's the IFF Data Specification record that contains one or more IFF data records.

class IffDataSpecification( object ):

It's one that has a null() too.

For IFF data, I'm seeing an issue with the code when I read the spec. The record length isn't for the data portion of the record, it's for the whole record including the type, length, data and padding.

image

@leif81 leif81 marked this pull request as draft June 4, 2020 04:22
@leif81
Copy link
Member Author

leif81 commented Jul 23, 2020

There are more nulls to fix, but the nulls fixed in this PR are still useful so I'm going to merge this as is since I've been busy and not sure when I will get back to fixing the rest.

@leif81 leif81 marked this pull request as ready for review July 23, 2020 13:00
@leif81 leif81 merged commit 23bba55 into master Jul 23, 2020
@leif81 leif81 deleted the leif81-null branch July 23, 2020 13:00
@leif81 leif81 linked an issue Jan 13, 2021 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

null() Function Call?
2 participants