From 77c358ed6732907c1bba44563353abe75ca2d1ec Mon Sep 17 00:00:00 2001 From: lord Date: Wed, 9 Sep 2020 11:35:52 -0700 Subject: [PATCH] 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 | 2 + d2common/d2loader/asset/source.go | 4 +- d2common/d2loader/filesystem/asset.go | 5 ++ d2common/d2loader/filesystem/source.go | 7 ++- d2common/d2loader/loader.go | 59 ++++++++++--------- d2common/d2loader/loader_test.go | 78 +++++++++++++++++--------- d2common/d2loader/mpq/asset.go | 9 ++- d2common/d2loader/mpq/source.go | 7 ++- 8 files changed, 113 insertions(+), 58 deletions(-) diff --git a/d2common/d2loader/asset/asset.go b/d2common/d2loader/asset/asset.go index b758a43c..081caa09 100644 --- a/d2common/d2loader/asset/asset.go +++ b/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 diff --git a/d2common/d2loader/asset/source.go b/d2common/d2loader/asset/source.go index a6ee129b..ddfcd821 100644 --- a/d2common/d2loader/asset/source.go +++ b/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 } diff --git a/d2common/d2loader/filesystem/asset.go b/d2common/d2loader/filesystem/asset.go index b86a7ecf..2eb2e4c4 100644 --- a/d2common/d2loader/filesystem/asset.go +++ b/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() +} diff --git a/d2common/d2loader/filesystem/source.go b/d2common/d2loader/filesystem/source.go index 230ecbca..af366560 100644 --- a/d2common/d2loader/filesystem/source.go +++ b/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() +} diff --git a/d2common/d2loader/loader.go b/d2common/d2loader/loader.go index 042b59de..46c36155 100644 --- a/d2common/d2loader/loader.go +++ b/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) } diff --git a/d2common/d2loader/loader_test.go b/d2common/d2loader/loader_test.go index 573a85af..91b82e0b 100644 --- a/d2common/d2loader/loader_test.go +++ b/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"}, diff --git a/d2common/d2loader/mpq/asset.go b/d2common/d2loader/mpq/asset.go index 8692a4e0..45c65933 100644 --- a/d2common/d2loader/mpq/asset.go +++ b/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() +} diff --git a/d2common/d2loader/mpq/source.go b/d2common/d2loader/mpq/source.go index e14af615..375d57b7 100644 --- a/d2common/d2loader/mpq/source.go +++ b/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() +}