-
Notifications
You must be signed in to change notification settings - Fork 147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix material count in material navigation popup #4227
base: prepare/150.0.x
Are you sure you want to change the base?
Conversation
This PR has Quantification details
Why proper sizing of changes matters
Optimal pull request sizes drive a better predictable PR flow as they strike a
What can I do to optimize my changes
How to interpret the change counts in git diff output
Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email) |
if (itemCount.Contains(".") && float.TryParse(itemCount, out var floatCount)) | ||
itemCount = $"{floatCount:0.####}"; | ||
itemCountText.text = L10nManager.Localize("UI_COUNT_FORMAT", itemCount); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (itemCount.Contains(".") && float.TryParse(itemCount, out var floatCount)) | |
itemCount = $"{floatCount:0.####}"; | |
itemCountText.text = L10nManager.Localize("UI_COUNT_FORMAT", itemCount); | |
itemCountText.text = L10nManager.Localize("UI_COUNT_FORMAT", itemCount); |
그냥 itemCount를 split하거나 하지 않고 바로 쓰면 안되나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
소수점이 엄청 길어지는 상황이 생기고 이것이 보기 안 좋을 수 있어서 4자리로 제한을 뒀습니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
어프루브를 누르긴 했는데, 생각해보니 float 말고 double이나 decimal로 TryParse 하는게 좋겠습니다. 여기 들어갈 값들이면 전부 원본 값이 long 아니면 BigInteger일거라서...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아래 현님의 사례처럼 소수점이 길어지는 상황이 생기더라도 이 팝업의 역할이 재화의 값을 자세히 표시하는 거라 itemCount 그대로 사용해도 되지 않을까 싶긴 합니다. 재화 값이 그렇게 극단적으로 길어지는 경우가 거의 없기도 하구요...
이거 crystal 개수에도 영향을 줄까요? crystal 은 소수점 아래로 18자리까지 있어서 잘못하면 지옥도가 펼쳐질 수 있습니다.
이런게 존재할 수 있습니다. |
일단은 소수점 4자리까지만 출력하도록 제한하였습니다 |
var split = itemCount.Split('.'); | ||
itemCountText.text = L10nManager.Localize("UI_COUNT_FORMAT", split[0]); | ||
|
||
if (itemCount.Contains(".") && float.TryParse(itemCount, out var floatCount)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
float 말고 double이나 decimal로 TryParse 하는게 좋겠습니다. 여기 들어갈 값들이면 전부 원본 값이 long 아니면 BigInteger일거라서...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수정된 코드의 클래스 이름을 봐서는 NCG에만 쓰이는게 아니라 다른 재화들에도 사용이 되고, 이 경우엔 소수점표시가 필요없는데도 소수점이 표시된다던지할 가능성이 있어보이는데 괜찮을까요?
출력 조건을 명확히 확인해보도록 하겠습니다 |
재화 타입에 따라 구분해서 NCG, Crystal 같이 소수점 표시가 필요한 재화만 GetQuantityString()으로 받아오고 있습니다. |
Description
재화 popup ui에 소수점이 있는 경우 무시하지 않고 보여줍니다
How to test
화면 상단의 소지중인 재화 혹은 포인트 UI를 클릭해서 팝업창을 확인합니다
Related Links
#4212