d2loader refactor (#722)

* d2loader refactor

- export sources
- asset and source both have a Path method
- make asset and source implement fmt.Stringer, which just calls Path method
- Loader.AddSource now returns the source and an error

* Update loader.go

d2common/d2loader/asset/asset.go

@@ -1,6 +1,7 @@
 package asset
 
 import (
+	"fmt"
 	"io"
 
 	"github.com/OpenDiablo2/OpenDiablo2/d2common/d2loader/asset/types"
@@ -9,6 +10,7 @@ import (
 // Asset represents a game asset. It has a type, an asset source, a sub-path (within the
 // asset source), and it can read data and seek within the data
 type Asset interface {
+	fmt.Stringer
 	io.ReadSeeker
 	Type() types.AssetType
 	Source() Source

d2common/d2loader/asset/source.go

@@ -1,12 +1,14 @@
 package asset
 
 import (
+	"fmt"
 	"github.com/OpenDiablo2/OpenDiablo2/d2common/d2loader/asset/types"
 )
 
 // Source is an abstraction for something that can load and list assets
 type Source interface {
+	fmt.Stringer
 	Type() types.SourceType
 	Open(name string) (Asset, error)
-	String() string
+	Path() string
 }

d2common/d2loader/filesystem/asset.go

@@ -42,3 +42,8 @@ func (fsa *Asset) Read(p []byte) (n int, err error) {
 func (fsa *Asset) Seek(offset int64, whence int) (int64, error) {
 	return fsa.file.Seek(offset, whence)
 }
+
+// String returns the path
+func (fsa *Asset) String() string {
+	return fsa.Path()
+}

d2common/d2loader/filesystem/source.go

@@ -44,6 +44,11 @@ func (s *Source) fullPath(subPath string) string {
 }
 
 // String returns the Root dir of this file system source
-func (s *Source) String() string {
+func (s *Source) Path() string {
 	return s.Root
 }
+
+// String returns the path
+func (s *Source) String() string {
+	return s.Path()
+}

d2common/d2loader/loader.go

@@ -17,9 +17,9 @@ import (
 )
 
 const (
-	defaultCacheBudget = 1024 * 1024 * 512
+	defaultCacheBudget      = 1024 * 1024 * 512
 	defaultCacheEntryWeight = 1
-	errFileNotFound = "file not found"
+	errFileNotFound         = "file not found"
 )
 
 // NewLoader creates a new loader
@@ -30,12 +30,12 @@ func NewLoader() *Loader {
 	return loader
 }
 
-// Loader represents the manager that handles loading and caching assets with the asset sources
+// Loader represents the manager that handles loading and caching assets with the asset Sources
 // that have been added
 type Loader struct {
 	d2interface.Cache
 	*d2util.Logger
-	sources []asset.Source
+	Sources []asset.Source
 }
 
 // Load attempts to load an asset with the given sub-path. The sub-path is relative to the root
@@ -50,13 +50,13 @@ func (l *Loader) Load(subPath string) (asset.Asset, error) {
 	}
 
 	// if it isn't in the cache, we check if each source can open the file
-	for idx := range l.sources {
-		source := l.sources[idx]
+	for idx := range l.Sources {
+		source := l.Sources[idx]
 
 		// if the source can open the file, then we cache it and return it
 		if loadedAsset, err := source.Open(subPath); err == nil {
-			l.Insert(subPath, loadedAsset, defaultCacheEntryWeight)
-			return loadedAsset, nil
+			err := l.Insert(subPath, loadedAsset, defaultCacheEntryWeight)
+			return loadedAsset, err
 		}
 	}
 
@@ -67,9 +67,9 @@ func (l *Loader) Load(subPath string) (asset.Asset, error) {
 // or a file on the host filesystem. In the case that it is a file, the file extension is used
 // to determine the type of asset source. In the case that the path points to a directory, a
 // FileSystemSource will be added.
-func (l *Loader) AddSource(path string) {
-	if l.sources == nil {
-		l.sources = make([]asset.Source, 0)
+func (l *Loader) AddSource(path string) (asset.Source, error) {
+	if l.Sources == nil {
+		l.Sources = make([]asset.Source, 0)
 	}
 
 	cleanPath := filepath.Clean(path)
@@ -77,38 +77,43 @@ func (l *Loader) AddSource(path string) {
 	info, err := os.Lstat(cleanPath)
 	if err != nil {
 		l.Warning(err.Error())
-		return
+		return nil, err
 	}
 
 	mode := info.Mode()
 
+	sourceType := types.AssetSourceUnknown
+
 	if mode.IsDir() {
-		source := &filesystem.Source{
-			Root: cleanPath,
-		}
-
-		l.Debug(fmt.Sprintf("adding filesystem source `%s`", cleanPath))
-		l.sources = append(l.sources, source)
+		sourceType = types.AssetSourceFileSystem
 	}
 
-	if !mode.IsRegular() {
-		return
+	if mode.IsRegular() {
+		ext := filepath.Ext(cleanPath)
+		sourceType = types.Ext2SourceType(ext)
 	}
 
-	ext := filepath.Ext(cleanPath)
-	sourceType := types.Ext2SourceType(ext)
-
 	switch sourceType {
 	case types.AssetSourceMPQ:
 		source, err := mpq.NewSource(cleanPath)
 		if err == nil {
 			l.Debug(fmt.Sprintf("adding MPQ source `%s`", cleanPath))
-			l.sources = append(l.sources, source)
+			l.Sources = append(l.Sources, source)
+
+			return source, nil
 		}
+	case types.AssetSourceFileSystem:
+		source := &filesystem.Source{
+			Root: cleanPath,
+		}
+
+		l.Debug(fmt.Sprintf("adding filesystem source `%s`", cleanPath))
+		l.Sources = append(l.Sources, source)
+
+		return source, nil
 	case types.AssetSourceUnknown:
 		l.Warning(fmt.Sprintf("unknown asset source `%s`", cleanPath))
-		fallthrough
-	default:
-		return
 	}
+
+	return nil, fmt.Errorf("unknown asset source `%s`", cleanPath)
 }

d2common/d2loader/loader_test.go

@@ -8,16 +8,17 @@ import (
 )
 
 const (
-	sourceA     = "testdata/A"
-	sourceB     = "testdata/B"
-	sourceC     = "testdata/C"
-	sourceD     = "testdata/D.mpq"
-	commonFile  = "common.txt"
-	exclusiveA  = "exclusive_a.txt"
-	exclusiveB  = "exclusive_b.txt"
-	exclusiveC  = "exclusive_c.txt"
-	exclusiveD  = "exclusive_d.txt"
-	badFilePath = "a/bad/file/path.txt"
+	sourcePathA   = "testdata/A"
+	sourcePathB   = "testdata/B"
+	sourcePathC   = "testdata/C"
+	sourcePathD   = "testdata/D.mpq"
+	commonFile    = "common.txt"
+	exclusiveA    = "exclusive_a.txt"
+	exclusiveB    = "exclusive_b.txt"
+	exclusiveC    = "exclusive_c.txt"
+	exclusiveD    = "exclusive_d.txt"
+	badSourcePath = "/x/y/z.mpq"
+	badFilePath   = "a/bad/file/path.txt"
 )
 
 func TestLoader_NewLoader(t *testing.T) {
@@ -31,38 +32,63 @@ func TestLoader_NewLoader(t *testing.T) {
 func TestLoader_AddSource(t *testing.T) {
 	loader := NewLoader()
 
-	loader.AddSource(sourceA)
-	loader.AddSource(sourceB)
-	loader.AddSource(sourceC)
-	loader.AddSource(sourceD)
-	loader.AddSource("bad/path")
+	sourceA, errA := loader.AddSource(sourcePathA)
+	sourceB, errB := loader.AddSource(sourcePathB)
+	sourceC, errC := loader.AddSource(sourcePathC)
+	sourceD, errD := loader.AddSource(sourcePathD)
+	sourceE, errE := loader.AddSource(badSourcePath)
 
-	if loader.sources[0].String() != sourceA {
+	if errA != nil {
+		t.Error(errA)
+	}
+
+	if errB != nil {
+		t.Error(errB)
+	}
+
+	if errC != nil {
+		t.Error(errC)
+	}
+
+	if errD != nil {
+		t.Error(errD)
+	}
+
+	if errE == nil {
+		t.Error("expecting error on bad file path")
+	}
+
+	if sourceA.String() != sourcePathA {
 		t.Error("source path not the same as what we added")
 	}
 
-	if loader.sources[1].String() != sourceB {
+	if sourceB.String() != sourcePathB {
 		t.Error("source path not the same as what we added")
 	}
 
-	if loader.sources[2].String() != sourceC {
+	if sourceC.String() != sourcePathC {
 		t.Error("source path not the same as what we added")
 	}
 
-	if loader.sources[3].String() != sourceD {
+	if sourceD.String() != sourcePathD {
 		t.Error("source path not the same as what we added")
 	}
+
+
+	if sourceE != nil {
+		t.Error("source for bad path should be nil")
+	}
 }
 
 func TestLoader_Load(t *testing.T) {
 	loader := NewLoader()
 
-	loader.AddSource(sourceB) // we expect files common to any source to come from here
-	loader.AddSource(sourceD)
-	loader.AddSource(sourceA)
-	loader.AddSource(sourceC)
+	_, _ = loader.AddSource(sourcePathB) // we expect files common to any source to come from here
+	_, _ = loader.AddSource(sourcePathD)
+	_, _ = loader.AddSource(sourcePathA)
+	_, _ = loader.AddSource(sourcePathC)
 
-	entryCommon, errCommon := loader.Load(commonFile) // common file exists in all three sources
+	entryCommon, errCommon := loader.Load(commonFile) // common file exists in all three Sources
 
 	entryA, errA := loader.Load(exclusiveA) // each source has a file exclusive to itself
 	entryB, errB := loader.Load(exclusiveB)
@@ -73,7 +99,7 @@ func TestLoader_Load(t *testing.T) {
 
 	if entryCommon == nil || errCommon != nil {
 		t.Error("common entry should exist")
-	} else if entryCommon.Source() != loader.sources[0] {
+	} else if entryCommon.Source() != loader.Sources[0] {
 		t.Error("common entry should come from the first loader source")
 	}
 
@@ -93,7 +119,7 @@ func TestLoader_Load(t *testing.T) {
 		entry asset.Asset
 		data  string
 	}{
-		{entryCommon, "b"}, // sourceB is loaded first, we expect a "b"
+		{entryCommon, "b"}, // sourcePathB is loaded first, we expect a "b"
 		{entryA, "a"},
 		{entryB, "b"},
 		{entryC, "c"},

d2common/d2loader/mpq/asset.go

@@ -14,7 +14,7 @@ var _ asset.Asset = &Asset{}
 // Asset represents a file record within an MPQ archive
 type Asset struct {
 	stream d2interface.ArchiveDataStream
-	name   string
+	path   string
 	source *Source
 }
 
@@ -30,7 +30,7 @@ func (a *Asset) Source() asset.Source {
 
 // Path returns the sub-path (within the source) of this asset
 func (a *Asset) Path() string {
-	return a.name
+	return a.path
 }
 
 // Read will read asset data into the given buffer
@@ -42,3 +42,8 @@ func (a *Asset) Read(buf []byte) (n int, err error) {
 func (a *Asset) Seek(offset int64, whence int) (n int64, err error) {
 	return a.stream.Seek(offset, whence)
 }
+
+// Path returns the path
+func (a *Asset) String() string {
+	return a.Path()
+}

d2common/d2loader/mpq/source.go

@@ -47,6 +47,11 @@ func (v *Source) Open(name string) (a asset.Asset, err error) {
 }
 
 // String returns the path of the MPQ on the host filesystem
-func (v *Source) String() string {
+func (v *Source) Path() string {
 	return v.MPQ.Path()
 }
+
+// String returns the path
+func (v *Source) String() string {
+	return v.Path()
+}