Fill logics into CustomEquipmentCraft action #2707
Conversation
- Required level is managed at ItemRequirementSheet. - We give different ItemId by proficiency group and can manage required level from this.
- Add `ratio` field to represent ratio to pick this row - Update OptionSheet model to make easier to save and handle substats
- Move functions to ItemFactory
- SelectIcon
- SelectOption
- SelectSkill
- CreateCustomEquipment
- Set subRecipeId in mail to 0: It's useless
- We want to get EquipmentItemOptionSheet data by id, not skillID. - Column name "skillId" can confuse user
|
idun setting 을 위해 사후 리뷰로 넘깁니다 cc. @ipdae |
| public const int DrawingItemId = 600401; | ||
| public const int DrawingToolItemId = 600402; |
There was a problem hiding this comment.
이건 별도 MaterialItemSheet를 사용하지 않는걸까요? 추가로 해머에선 놓쳤는데 이 아이템들은 별도의 ItemSubType을 지정해두는게 좋을것 같습니다.
There was a problem hiding this comment.
MaterialItemSheet 에 있기는 한데 ItemId 가 고정이어서 넣어놨습니다. 어차피 sheet 에서 불러도 저 ID 로 부를거라 차이 없을거라 생각했습니다.
- 도면이랑 제도도구의 subtype 이 두개 생겨야 할까요? 사실 이 subtype 의 용처가 확 와닿지 않아서..
| // Validate Materials | ||
| // Should finalize cost using sheet, additional cost and proficiency | ||
| // Modify cost to get real cost | ||
| var requiredFungibleItems = new Dictionary<int, int>(); |
There was a problem hiding this comment.
아래까지의 코드를 봐선 우선 필요 재료들을 모아둔 다음 removeMaterial은 loop안에서 처리하는게 코드를 파악하기 용이할것같습니다.
There was a problem hiding this comment.
네 안그래도 비용 계산을 helper 로 빼기로 해서 같이 정리될 것으로 보입니다.
| public decimal CostMultiplier { get; private set; } | ||
| public decimal RequiredBlockMultiplier { get; private set; } |
There was a problem hiding this comment.
이건 타입이 decimal이어야할 이유가 있는걸까요? 코드를 봐선 long타입이 더 맞는것같습니다.
There was a problem hiding this comment.
다른 multiplier들도 가능하면 decimal대신 만분율을 적용한다던지 하는식으로 단순화시키는게 좋을것같습니다.
There was a problem hiding this comment.
1.2배, 1.5배 같은걸 생각했는데, 만분율은 논의해보겠습니다.
| return optionSelector.Select(1).First(); | ||
| } | ||
|
|
||
| public static Skill.Skill SelectSkill( |
There was a problem hiding this comment.
위치가 SkillFactory에 있는게 더 알맞을것 같습니다.
| ); | ||
|
|
||
| // Create equipment with ItemFactory | ||
| var equipment = ItemFactory.CreateCustomEquipment( |
There was a problem hiding this comment.
CreateCustomEquipment의 경우를 제외하고 위에서 장비 옵션관련으로 불리는 메서드들이 사용되는곳이 있을까요? 만약 없다면 관련 처리들은 이 메서드 안에서 해도 될것같습니다.
| return cp / 1m; | ||
| case StatType.NONE: | ||
| default: | ||
| // throw new ArgumentOutOfRangeException(nameof(statType), statType, null); |
|
여기 있는 review 내역에 대한 반영은 별도 PR 로 진행합니다. |
Add logic for CustomEquipmentCraft action.
Review Point