refactor: support multi-column ordered index, optimize code#139
Merged
refactor: support multi-column ordered index, optimize code#139
Conversation
Kybxd
force-pushed
the
multi-ordered-index
branch
from
5bd8097 to
810d61b
Compare
Kybxd
force-pushed
the
multi-ordered-index
branch
from
68c628f to
f11b8f9
Compare
wenchy
requested changes
Nov 17, 2025
Comment on lines
127
to
+143
| @@ -131,6 +138,9 @@ func ParseOrderedMapKeyType(fd protoreflect.FieldDescriptor) string { | |||
| return "double" | |||
| case protoreflect.StringKind: | |||
| return "std::string" | |||
| case protoreflect.EnumKind: | |||
| protoFullName := string(fd.Enum().FullName()) | |||
| return strings.ReplaceAll(protoFullName, ".", "::") | |||
Member
There was a problem hiding this comment.
The Map, OrderedMap, Index, and OrderedIndex all should process the enum key in the same way.
Collaborator
Author
There was a problem hiding this comment.
Only for ordered index currently, so rename this function to ParseOrderedIndexKeyType.
| } | ||
| } | ||
| keys = append(keys, MapKey{ParseMapKeyType(fd.MapKey()), name}) | ||
| keys = append(keys, newKey) |
Member
There was a problem hiding this comment.
Should not write back to slice receiver, as it's error-prone.
| @@ -0,0 +1,306 @@ | |||
| package orderedindex | |||
Member
There was a problem hiding this comment.
Rename dir to be consistent with package name:
ordered_index->orderedindexordered_map->orderedmap
| } | ||
| } | ||
|
|
||
| func (x *Generator) Generate() bool { |
Comment on lines
+36
to
+44
| using OrderedMap_Hero_AttrMap = std::map<std::string, const protoconf::HeroConf::Hero::Attr*>; | ||
| const OrderedMap_Hero_AttrMap* GetOrderedMap(const std::string& name) const; | ||
|
|
||
| using Hero_OrderedMapValue = std::pair<Hero_Attr_OrderedMap, const protoconf::HeroConf::Hero*>; | ||
| using Hero_OrderedMap = std::map<std::string, Hero_OrderedMapValue>; | ||
| const Hero_OrderedMap* GetOrderedMap() const; | ||
| using OrderedMap_HeroValue = std::pair<OrderedMap_Hero_AttrMap, const protoconf::HeroConf::Hero*>; | ||
| using OrderedMap_HeroMap = std::map<std::string, OrderedMap_HeroValue>; | ||
| const OrderedMap_HeroMap* GetOrderedMap() const; | ||
|
|
||
| private: | ||
| Hero_OrderedMap ordered_map_; | ||
| OrderedMap_HeroMap ordered_map_; |
Member
There was a problem hiding this comment.
Should not change this. Keep compatible, and its naming is ok.
Member
There was a problem hiding this comment.
Naming convention: <Package>_<Message>_[OrderedMap|Index|OrederedIndex]_[Levels]
| // Index types. | ||
| // Index: Title | ||
| type HeroConf_Index_AttrMap = map[string][]*protoconf.HeroConf_Hero_Attr | ||
| type Index_HeroConf_AttrMap = map[string][]*protoconf.HeroConf_Hero_Attr |
Member
There was a problem hiding this comment.
The original name means in the namespace HeroConf.
Comment on lines
+97
to
+106
| indexItemDefaultInfoMap Index_ItemConf_ItemDefaultInfoMap | ||
| indexItemExtInfoMap Index_ItemConf_ItemExtInfoMap | ||
| indexAwardItemMap Index_ItemConf_AwardItemMap | ||
| indexSpecialItemMap Index_ItemConf_SpecialItemMap | ||
| indexItemPathDirMap Index_ItemConf_ItemPathDirMap | ||
| indexItemPathNameMap Index_ItemConf_ItemPathNameMap | ||
| indexItemPathFriendIdMap Index_ItemConf_ItemPathFriendIDMap | ||
| indexUseEffectTypeMap Index_ItemConf_UseEffectTypeMap | ||
| orderedIndexExtTypeMap *OrderedIndex_ItemConf_ExtTypeMap | ||
| orderedIndexParamExtTypeMap *OrderedIndex_ItemConf_ParamExtTypeMap |
Member
There was a problem hiding this comment.
The original naming is more better and self-evident.
wenchy
approved these changes
Nov 17, 2025
Comment on lines
+36
to
+44
| using OrderedMap_Hero_AttrMap = std::map<std::string, const protoconf::HeroConf::Hero::Attr*>; | ||
| const OrderedMap_Hero_AttrMap* GetOrderedMap(const std::string& name) const; | ||
|
|
||
| using Hero_OrderedMapValue = std::pair<Hero_Attr_OrderedMap, const protoconf::HeroConf::Hero*>; | ||
| using Hero_OrderedMap = std::map<std::string, Hero_OrderedMapValue>; | ||
| const Hero_OrderedMap* GetOrderedMap() const; | ||
| using OrderedMap_HeroValue = std::pair<OrderedMap_Hero_AttrMap, const protoconf::HeroConf::Hero*>; | ||
| using OrderedMap_HeroMap = std::map<std::string, OrderedMap_HeroValue>; | ||
| const OrderedMap_HeroMap* GetOrderedMap() const; | ||
|
|
||
| private: | ||
| Hero_OrderedMap ordered_map_; | ||
| OrderedMap_HeroMap ordered_map_; |
Member
There was a problem hiding this comment.
Naming convention: <Package>_<Message>_[OrderedMap|Index|OrederedIndex]_[Levels]
wenchy
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.