Custom field stuff #75
Conversation
| _binWriter.Write(def.DisplayText); | ||
| } | ||
| // signal end of custom field definitions | ||
| _binWriter.Write(""); |
There was a problem hiding this comment.
instead of an empty string marker, how about writing a number of custom fields ahead of time? Reader won't have to guess how many custom fields there will be, much like it's done for beatmaps & beatmaphashes.
| // completely skip the custom fields section if there are no definitions in the header of this collection | ||
| if(customFieldDefLookup.Count > 0) | ||
| { | ||
| foreach (var kvp in ((BeatmapExtension)beatmap).GetAllCustomFields()) | ||
| { | ||
| if (!customFieldDefLookup.TryGetValue(kvp.Key, out var def) || | ||
| !CustomFieldWriters.TryGetValue(def.Type, out var writer)) | ||
| { | ||
| // definition or its stream writer not found??? skip value I guess | ||
| continue; | ||
| } | ||
|
|
||
| _binWriter.Write(kvp.Key); | ||
| writer(_binWriter, kvp.Value); | ||
| } | ||
| // signal end of custom field values | ||
| _binWriter.Write(""); | ||
| } |
There was a problem hiding this comment.
Same request as before - write number of custom fields definitions ahead of time
| @@ -238,19 +293,45 @@ public IEnumerable<Collection> ReadOsdb(string fullFileDir, MapCacher mapCacher) | |||
| map.UserComment = _binReader.ReadString(); | |||
| } | |||
|
|
|||
| if (fileVersion >= 8 || (fileVersion >= 5 && IsFullCollection(versionString))) | |||
| //versions 5-7 field existed only full collection, since version 8 field is always here | |||
There was a problem hiding this comment.
correcting my own typos
| //versions 5-7 field existed only full collection, since version 8 field is always here | |
| //versions 5-7 field existed only in full collections, since version 8 field is always here |
| { | ||
| map.PlayMode = (PlayMode)_binReader.ReadByte(); | ||
| } | ||
|
|
||
| if (fileVersion >= 8 || (fileVersion >= 6 && IsFullCollection(versionString))) | ||
| //versions 6-7 field existed only full collection, since version 8 field is always here |
There was a problem hiding this comment.
correcting my own typos
| //versions 6-7 field existed only full collection, since version 8 field is always here | |
| //versions 6-7 field existed only in full collections, since version 8 field is always here |
| if (!customFieldDefLookup.TryGetValue(kvp.Key, out var def) || | ||
| !CustomFieldWriters.TryGetValue(def.Type, out var writer)) | ||
| { | ||
| // definition or its stream writer not found??? skip value I guess |
There was a problem hiding this comment.
| // definition or its stream writer not found??? skip value I guess | |
| // skip value if definition or its stream writer doesn't exist |
| _binWriter.Write(def.DisplayText); | ||
| } | ||
| // signal end of custom field definitions | ||
| _binWriter.Write(""); |
There was a problem hiding this comment.
All collections should share definitions in osdb - how about storing these once at the beginning?
| } | ||
| } | ||
|
|
||
| public IEnumerable<KeyValuePair<string, object>> GetAllCustomFields() |
There was a problem hiding this comment.
| public IEnumerable<KeyValuePair<string, object>> GetAllCustomFields() | |
| public IEnumerable<KeyValuePair<string, object>> GetCustomFields() |
| return _customFields.TryGetValue(key, out var value) ? value : null; | ||
| } | ||
|
|
||
| public IEnumerable<string> GetAllStringCustomFieldValues() |
There was a problem hiding this comment.
| public IEnumerable<string> GetAllStringCustomFieldValues() | |
| public IEnumerable<string> GetStringCustomFieldValues() |
|
|
||
| public IEnumerable<KeyValuePair<string, object>> GetAllCustomFields() | ||
| { | ||
| return _customFields; |
There was a problem hiding this comment.
| return _customFields; | |
| return _customFields ?? Enumerable.Empty<KeyValuePair<string, object>>(); |
omkelderman
force-pushed
the
custom-field-stuff
branch
from
a4fb0ae to
e49d4b4
Compare
|
all feedback should be included except
This one I havent worked in yet. Currently each Collection has its own list of custom field definitions, that is how I originally also designed the system with my custom website where you could download multiple osdb each containing one collection with their own set of custom field definitions. Then when all of them have been loaded into CM if you switch between the different collections it will dynamically add or remove the columns in the view as needed. I get what ur saying, but that also means that the read and write code needs significant update cuz with this idea the list of custom field definitions is no longer tied to a single Collection but something global and then you need to consider stuff like ok what if in ur global definitions list is something with key X and then you load a new file that also has the same key X but a different type, what do you do? With the current system that is no problem cuz each collection has its own set of definitions that can be different from each other. Now all that said I do remember where you said that all maps share data between collections so this will be a problem that needs to be addressed either way as "With the current system that is no problem" in my sentence above then applies to the definitions but not the values and that will probably break. Either way that is a problem yet to be solved so guess this PR is still WIP but that is something for another day |
No description provided.