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 Exceptions in VocLoader #15975

Merged
merged 1 commit into from May 15, 2019

Conversation

@evgeniysergeev
Copy link
Contributor

commented Dec 31, 2018

VocLoader in OpenRA do not work because of System.IO.InvalidDataException.
I think it was broken by Enumerators changes in .Net.

The line:
currentBlock = (IEnumerator<VocBlock>)blocks.GetEnumerator();
produce System.IO.InvalidDataException
and should be changed to this:
currentBlock = ((IEnumerable<VocBlock>)blocks).GetEnumerator();

and
blocks.Last()
also produce exception System.InvalidOperationException: Enumeration already finished, so it should be in try/catch block.

@evgeniysergeev

This comment has been minimized.

Copy link
Contributor Author

commented Dec 31, 2018

This PR need for OpenRA/d2#89

@pchote

This comment has been minimized.

Copy link
Member

commented Jan 1, 2019

I think it was broken by Enumerators changes in .Net.

It seems unlikely that Microsoft would intentionally introduce a breaking change like this, and @IceReaper reports that the dune2 vocs work fine with the current loader. Can you please describe your compiler and configuration that triggers the crashes?

@evgeniysergeev

This comment has been minimized.

Copy link
Contributor Author

commented Jan 8, 2019

on Windows:

OpenRA engine version release-20181215
d2 mod mod version release-20181215
Date: 2019-01-08 06:59:18Z
Operating System: Windows (Microsoft Windows NT 6.1.7601 Service Pack 1)
Runtime Version: .NET CLR 4.0.30319.42000
Exception of type `System.IO.InvalidDataException`: BUTTON.VOC is not a valid sound file!

dotnetversion

@evgeniysergeev

This comment has been minimized.

Copy link
Contributor Author

commented Jan 8, 2019

On CentOS 7 Linux:

Platform is Linux
Engine version is release-20181215
Using SDL 2 with OpenGL renderer
Desktop resolution: 1920x1200
No custom resolution provided, using desktop resolution
Using resolution: 1920x1200
Using window scale 1.00
OpenGL version: 3.0 Mesa 17.2.3
Using default sound device
Internal mods:
	d2: d2 mod (release-20181215)
	all: All mods (release-20181215)
	cnc: Tiberian Dawn (release-20181215)
	d2k: Dune 2000 (release-20181215)
	modcontent: Mod Content Manager (release-20181215)
	ra: Red Alert (release-20181215)
	ts: Tiberian Sun (release-20181215)
External mods:
	d2-release-20181215: d2 mod (release-20181215)
	d2k-release-20181215: Dune 2000 (release-20181215)
Loading mod: d2
Exception of type `System.IO.InvalidDataException`: BUTTON.VOC is not a valid sound file!
$ mono --version
Mono JIT compiler version 5.18.0.225 (tarball Thu Jan  3 08:42:34 UTC 2019)
Copyright (C) 2002-2014 Novell, Inc, Xamarin Inc and Contributors. www.mono-project.com
	TLS:           __thread
	SIGSEGV:       altstack
	Notifications: epoll
	Architecture:  amd64
	Disabled:      none
	Misc:          softdebug 
	Interpreter:   yes
	LLVM:          yes(600)
	Suspend:       preemptive
	GC:            sgen (concurrent by default)
$ rpm -qa | grep mono
mono-devel-5.18.0.225-0.xamarin.13.epel7.x86_64
@evgeniysergeev

This comment has been minimized.

Copy link
Contributor Author

commented Jan 8, 2019

On macOS 10.13.6

Platform is OSX
Engine version is release-20181215
Using SDL 2 with OpenGL renderer
Desktop resolution: 1920x1200
Using resolution: 1024x768
Using window scale 1.00
OpenGL version: 2.1 INTEL-10.4.14
Using default sound device
Internal mods:
	d2: d2 mod (release-20181215)
	all: All mods (release-20181215)
	cnc: Tiberian Dawn (release-20181215)
	d2k: Dune 2000 (release-20181215)
	modcontent: Mod Content Manager (release-20181215)
	ra: Red Alert (release-20181215)
	ts: Tiberian Sun (release-20181215)
External mods:
	d2-release-20181215: d2 mod (release-20181215)
	d2k-release-20180923: Dune 2000 (release-20180923)
Loading mod: d2
Exception of type `System.IO.InvalidDataException`: BUTTON.VOC is not a valid sound file!
$ mono --version
Mono JIT compiler version 5.16.0.221 (2018-06/b63e5378e38 Mon Nov 19 18:08:09 EST 2018)
Copyright (C) 2002-2014 Novell, Inc, Xamarin Inc and Contributors. www.mono-project.com
	TLS:           normal
	SIGSEGV:       altstack
	Notification:  kqueue
	Architecture:  amd64
	Disabled:      none
	Misc:          softdebug 
	Interpreter:   yes
	LLVM:          yes(3.6.0svn-mono-release_60/0b3cb8ac12c)
	GC:            sgen (concurrent by default)

@evgeniysergeev evgeniysergeev force-pushed the evgeniysergeev:VocLoader-endOfData branch from ee08884 to 30d9689 Jan 8, 2019

@evgeniysergeev

This comment has been minimized.

Copy link
Contributor Author

commented Jan 8, 2019

Added fix for VOC file version check from exact match VOC version 1.1 to any 1.x version.
This is also produce exception for some VOC files.
Already used here: OpenRA/d2#101

@evgeniysergeev

This comment has been minimized.

Copy link
Contributor Author

commented Jan 8, 2019

and @IceReaper reports that the dune2 vocs work fine with the current loader.

@pchote, @IceReaper if you have d2 mod installed and running, you can try to reproduce by simply replace

SoundFormats: D2Voc, Aud, Wav

with

SoundFormats: Voc, Aud, Wav

in mods/d2/mod.yaml

VocLoader from OpenRA engine will be used. if you will able to click on any button and hear BUTTON.VOC, then current loader is ok. But for me it don’t work on any platform

@abcdefg30

This comment has been minimized.

Copy link
Member

commented Jan 8, 2019

Can confirm that changing D2Voc to Voc crashes with the exception from above.

@abcdefg30

This comment has been minimized.

Copy link
Member

commented Mar 11, 2019

What is the status here? This looks like a workaround but does fix the issue.
(Can we check if the currentBlocks enumeration is finished without throwing and catching the exception?)

Fix for System.IO.InvalidDataException in VocLoader
Fix for VOC version check

use cached value from MoveNext instead of try/catch section

@evgeniysergeev evgeniysergeev force-pushed the evgeniysergeev:VocLoader-endOfData branch from 30d9689 to 4901472 May 14, 2019

@evgeniysergeev

This comment has been minimized.

Copy link
Contributor Author

commented May 14, 2019

What is the status here? This looks like a workaround but does fix the issue.
(Can we check if the currentBlocks enumeration is finished without throwing and catching the exception?)

The solution is to use last return value from MoveNext, because after it returns false, unable to call Current without throw exception. When currentBlocks created, currentBlocksEnded set to false. And set to true only if MoveNext was return false. This can happen only if execution not beaked in loops that uses MoveNext. So, it set after the while loop to true.
This implemented in updated PR. Checked that works with current d2 mod, that uses VOC files.

@evgeniysergeev evgeniysergeev force-pushed the evgeniysergeev:VocLoader-endOfData branch from 4901472 to 30772c8 May 14, 2019

@abcdefg30
Copy link
Member

left a comment

Fix confirmed.

evgeniysergeev added a commit to evgeniysergeev/OpenRA that referenced this pull request May 14, 2019
evgeniysergeev added a commit to evgeniysergeev/OpenRA that referenced this pull request May 14, 2019

@reaperrr reaperrr merged commit 3bc5b07 into OpenRA:bleed May 15, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@evgeniysergeev evgeniysergeev deleted the evgeniysergeev:VocLoader-endOfData branch May 18, 2019

@evgeniysergeev evgeniysergeev restored the evgeniysergeev:VocLoader-endOfData branch May 18, 2019

@evgeniysergeev evgeniysergeev deleted the evgeniysergeev:VocLoader-endOfData branch May 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.