Resolve #750 - Remove Explicit Interface Implementations#751
Resolve #750 - Remove Explicit Interface Implementations#751Veykril merged 12 commits intoLeagueSandbox:indevfrom Veykril:interface_fixes
Conversation
Codecov Report
@@ Coverage Diff @@
## indev #751 +/- ##
=======================================
+ Coverage 1.88% 1.9% +0.02%
=======================================
Files 131 131
Lines 7816 7627 -189
Branches 616 581 -35
=======================================
- Hits 147 145 -2
+ Misses 7669 7482 -187
Continue to review full report at Codecov.
|
danil179
left a comment
danil179
left a comment
There was a problem hiding this comment.
Some problematic hierarchy in the interfaces
Veykril
changed the title
Veykril
changed the title
Veykril
changed the title
There was a problem hiding this comment.
Approving with some questions, seems like most of hierarchy problems get solved, there is one more refactor PR that is going to be full of conflicts from this with some important change, I hope it get merged, I think this and #745 are the last huge refactoring for the moment.
| GetDistanceTo(u) > DETECT_RANGE || // in range | ||
| !_game.ObjectManager.TeamHasVisionOn(Team, u)) // visible to this minion | ||
| continue; // If not, look for something else | ||
| if (!(it.Value is IAttackableUnit u) || |
There was a problem hiding this comment.
Why did you removed the comments?
There was a problem hiding this comment.
cause to my eyes, the comments literally repeat what the code describes itself already pretty much
| public static Item CreateFromType(Inventory inventory, ItemType item) | ||
| public void SetStacks(byte newStacks) | ||
| { | ||
| throw new System.NotImplementedException(); |
There was a problem hiding this comment.
Open issue about that, need to be fixed fast.
There was a problem hiding this comment.
will do that, currently this function is only used by buffs which is why this doesnt matter too much in itself
| public int RecipeItem2 { get; private set; } | ||
| public int RecipeItem3 { get; private set; } | ||
| public int RecipeItem4 { get; private set; } | ||
| private int RecipeItem1 { get; set; } |
There was a problem hiding this comment.
This part is fixed in another PR so you don't need to do array here cause it already fixed there (we should open issue about that and comment that it already fixed in another unmerged PR).
There was a problem hiding this comment.
yea i changed it a bit only cause of the talk that resulted afterwards. pretty sure there is no open PR about this atm though
There was a problem hiding this comment.
oh i see, thats kind of offtopic for that pr i suppose. well ill keep it like this now though given that the zip PR isnt in mergeable state yet either so he can just overwite it with his
4 participants
Closes #750
Refs LeagueSandbox/LeagueSandbox-Default#85