[무역 상점] 인벤토리 및 판매목록 구현 #190

kth1888 · 2024-05-22T16:37:47Z

TL;DR

무역 상점 화면에 내 아이템과 상대방과 내가 올린 물품을 볼 수 있습니다.

내 판매목록 탭 (클릭 시) -

좌측에는 내 아이템이 보이고, 우측에는 상대방이 올린 물품을 기본적으로 보여줍니다.
탭을 변경 시 내가 올린 아이템만 볼 수 있습니다.
현재는 타인이 올린 아이템이 없어서 자신이 올린 아이템을 보여주는 예시입니다.

인벤토리가 탭 UI처럼 보이지 않도록 수정했습니다.
아이템은 이름/등급 별로 묶이고 이후 작업을 위한 아이템 등록 버튼을 추가했습니다.
판매목록과 내 판매목록의 기능이 다르므로 UI를 분리하여 자연스럽게 수정했습니다.
표시 정보 중에 등급을 추가했습니다.

부엌, 농장 화면에서 진입 가능하도록 변경했습니다.
제목과 무역 버튼을 자연스러운 이름으로 변경했습니다.

frontend/Savor-22b/gql/query_executor.gd

Atralupus · 2024-05-26T07:01:08Z

frontend/Savor-22b/scenes/market/market.gd

+signal query_received
+
+const TradeInventoryScn = preload("res://scenes/market/trade_inventory.tscn")
+const SellListScn = preload("res://scenes/market/sell_list.tscn")


변수명이랑 신 이름이 좀 이상한데 백엔드랑 용어 통일 할까요

Atralupus · 2024-05-26T07:02:14Z

frontend/Savor-22b/scenes/market/market.gd

+	add_child(register_trade_good_query_executor)
+	add_child(stage_tx_mutation_executor)
+	add_child(trade_inventory_state_executor)
+


띄어쓰기를 전체적으로 다 한칸씩만 하도록 해놨습니다. 파일 전체적으로 통일해주세요

frontend/Savor-22b/scenes/market/market.gd

frontend/Savor-22b/scenes/market/my_item.gd

frontend/Savor-22b/scenes/market/posted_item.gd

Atralupus · 2024-05-26T07:35:27Z

그리고 리베이스 필요합니다

Atralupus · 2024-05-26T11:53:00Z

그리고 무역은 마을에서 활동을 했어야 사용할 수 있으니 부엌, 농장쪽에 진입 버튼을 두는게 맞아보입니다

upa-r-upa · 2024-05-26T12:17:02Z

지나가는 길에 봤는데요. 일단 작업 고생하셨습니다.
코드는 오프 기간이라 안읽었고 그냥 사진과 PR에 올려진 기능 내용으로만 간단하게 리뷰 답니다.
문제점이 좀 있는데요, 지원님이 말씀해주신 건 제외하고 간단하게 적어봅니다..

일단 적어도 직접 틀어보지 않고 이 PR안에서 이 기능을 리뷰해드리려면 화면별 사진 정도는 전부 올라와야 한다고 생각하는데
당장 짧게만 봐도 내가 올린 판매 목록 탭은 PR에 포함이 안되어 있어서 어떻게 구현되어 있는지 코드로만 봐야 해서 불편합니다.
탭 동작 UI가 겉보기에 이상합니다.

보통 위처럼 생긴 탭은 배경 색과 선택한 하이라이트 색상이 같아야 하는데요.
반대로 되어 있어서 현재 선택된 탭이 무엇인지 엄청 헷갈립니다.
판매 목록 UI에서 품목명이 길어지면 잘리거나 ... 으로 처리될 것 같은데
저정도 여유 공간만 있다면.. 품목명에 따라 정상적으로 모두 표출되는 구간이 많이 짧은 것 같습니다.
인벤토리 UI에서도 마찬가지로 텍스트가 정상적으로 표출되는 구간이 짧습니다. 그리고 stateID는 지금 아예 화면상에서만 봐도 잘린 것 같은데요..
판매 목록 UI의 게시자도 마찬가지 입니다. 잘린 것 같습니다.

첨언하자면 전체적으로 기본적인 부분이라고 생각되는 것들이 많이 미흡한데.. 리뷰를 받기 전에 스스로 체크해주실 수 있는 부분이라고 생각합니다. 지금보다 더 챙겨주시면 좋을 것 같습니다.

upa-r-upa

수정하신거 확인 했습니다. 고생 하셨어요.
일단 말씀 드린 것 자체는 수정이 대부분 된 것 같은데,
다시 보니 더 눈에 보이는 것들이 있네요.

이제 보니 페이지 제목이 '무역 화면' 이네요. 최소한 무역 상점이나 다른 이름을 보여주어야 하지 않을까요. 기본적인 부분이라고 생각해서 조금 아쉽습니다.
마찬가지로 버튼 배치나, 복잡한 UI 개선은 뒤로 하더라도 '무역으로' 라는 버튼은 말이 좀 이상합니다.
그리고 내 판매 목록 UI와 일반 판매 목록 UI가 같은건 잘못됐습니다. 말씀 드린것처럼 기능에 대해 최소한으로는 고민해보고 만드시는 게 중요합니다. 일단 적어도 내가 판매할 아이템 목록에 구매 버튼이 뜨는 건 어색할 것 같습니다.
지금 내 판매목록 UI가 API만 붙이면 사용 가능한 화면이라 보이지 않습니다. 정말 필수적으로 필요한 기능이라고 보여지는 게 없습니다.
기본적으로 1.내가 올린 판매 물품을 확인하고, 2. 등록을 취소하는 기능은 되어야 하지 않을까요.
그리고 인벤토리에서 StateId가 필요한 이유가 있을까요? 같은 등급, 같은 아이템이라면 묶어서 보여주거나 하는 게 더 용이해 보입니다.
인벤토리는 탭 기능이 나중에 추가 될 예정이 아니라면.. 탭이 아닌데 탭 UI일 필요가 없어 보입니다.

upa-r-upa · 2024-05-27T16:36:04Z

수정 고생하셨습니다. 잘 수정 하셨습니다.
조금만 더 수정하면 될 것 같습니다.

'무역 상점' 이라고 페이지 명을 확정하셨으면, 버튼도 '무역 하러 가기'보다는 용어 통일을 해야할 것 같습니다. '무역 상점' 정도로 통일하면 될 것 같습니다.
자세히 보니 판매목록/내 판매목록 모두 아이템 등급이 표기가 안되고 있는 것 같습니다. 등급이 있는 음식이나 식재료는 등급이 표시 되어야 합니다.

Atralupus · 2024-05-28T06:51:11Z

확인되기 전에 resolved는 누르지 말아주세요

upa-r-upa · 2024-05-28T06:55:59Z

수정 고생하셨습니다. 제가 말씀 드린 내용들은 잘 수정 된 것 같네요.
하나 그냥 알려드리자면 commit message에 #number 와 같은 식으로 적는 건 issue나 pr 번호로 인식되니까 지양하시는 게 좋습니다.
또 웬만하면 ui problems와 같이 추상적이고 여러 내용이 묶인 커밋도 지양하시는 게 좋습니다. 커밋을 나눠서 내용에 맞추어 메시지를 작성해주시면 좋습니다.

Atralupus · 2024-05-28T06:52:59Z

frontend/Savor-22b/scenes/market/sell_list_button_group.tres

@@ -0,0 +1,3 @@
+[gd_resource type="ButtonGroup" format=3 uid="uid://di66p0jb8cdpk"]


sell_list 파일 이름에 아직 있습니다

Atralupus · 2024-05-28T06:53:06Z

frontend/Savor-22b/scenes/market/trade_inventory.gd

+		if(good["sellerAddress"] != my_address):
+			add_lists(good, false)
+
+func _on_my_sell_list_button_down():


Atralupus · 2024-05-28T06:53:10Z

frontend/Savor-22b/scenes/market/trade_inventory.gd

+	trade_goods_scene.set_info(good)
+	list_container.add_child(trade_goods_scene)
+
+func _on_whole_sell_list_button_down():


Atralupus · 2024-05-28T06:53:19Z

frontend/Savor-22b/scenes/market/trade_inventory.tscn

+[gd_scene load_steps=7 format=3 uid="uid://bu43d13qwmtcr"]
+
+[ext_resource type="Script" path="res://scenes/market/trade_inventory.gd" id="1_2sbin"]
+[ext_resource type="ButtonGroup" uid="uid://di66p0jb8cdpk" path="res://scenes/market/sell_list_button_group.tres" id="2_68j2i"]


Atralupus · 2024-05-28T06:53:28Z

frontend/Savor-22b/scenes/market/trade_inventory.tscn

+theme_override_constants/v_separation = 50
+columns = 2
+
+[connection signal="button_down" from="MarginContainer/VBoxContainer/HBoxContainer/WholeSellListButton" to="." method="_on_whole_sell_list_button_down"]


Atralupus · 2024-05-28T06:59:04Z

frontend/Savor-22b/scenes/market/my_trade_goods.tscn

+theme_override_colors/font_color = Color(1, 1, 1, 1)
+theme_override_font_sizes/font_size = 40
+theme_override_styles/normal = SubResource("StyleBoxFlat_lobix")
+text = "   가격 수정   "


패딩 이렇게 공백으로주지 마시고 실제 스타일 테마 오버라이드 하셔서 패딩 또는 마진 값 설정해주세요

Atralupus · 2024-05-28T07:00:14Z

frontend/Savor-22b/scenes/market/my_item.gd

@@ -0,0 +1,24 @@
+extends Control
+
+@onready var item_name_label = $Background/MarginContainer/VBoxContainer/NameLabel


필수 수정사항은 아닙니다만 노드들 이름에 굳이 VBox, Margin 이런 기능적인 이름들 다 제거해주시면 좋겠습니다

Atralupus · 2024-05-28T07:03:27Z

frontend/Savor-22b/scenes/market/market.gd

+		trade_inventory_container.add_child(trade_inventory)
+		trade_inventory.set_list(inventory_state)
+
+func props_only_query_action(query_executor): # query with no args


아직 이름이 명확하지 않습니다. 또 따로 함수로 존재할 필요가 없어보입니다.
인자로 받는 query_executor가 특정한 query_executor만 사용 가능해보이고 이벤트 리스너를 등록하는 것이기 때문에 해당 함수에서 진행할 이유 또한 없습니다. 결국 해주는건 run 을 실행해주는건데 이걸 따로 해주는 함수가 필요하진 않아보입니다.

Atralupus · 2024-05-28T07:08:35Z

frontend/Savor-22b/scenes/market/my_item.gd

+	if $Background == null:
+		return
+
+	item_name_label.text = info[0]["name"]


이부분은 파악해보질 못했는데 0 인덱스로 접근해도 괜찮은건가요? 또 변수명으로 보아 정보를 받아오는 것 같은데 왜 Array인가요?

Atralupus · 2024-05-28T07:12:36Z

frontend/Savor-22b/scenes/market/trade_inventory.gd

+func _ready():
+	pass
+
+func set_list(data: Array):


이부분까지는 크게 상관 없는데 항상 네이밍은 더 명확하게 해주세요
set_list 보단 지금 goods에 데이터를 set 해주시니까 set_goods가 더 맞아보입니다 또 load까지 진행하니 set_goods_and_load가 될텐데 set_goods만 두고 load는 따로 호출해주는게 맞겠습니다

kth1888 marked this pull request as ready for review May 26, 2024 03:14

kth1888 requested a review from a team as a code owner May 26, 2024 03:14

kth1888 requested review from Atralupus and upa-r-upa May 26, 2024 03:14

Atralupus assigned kth1888 May 26, 2024

Atralupus requested changes May 26, 2024

View reviewed changes

kth1888 added 3 commits May 26, 2024 18:52

feat: trade shop inventory implement

028e72a

feat: trade shop sell list implement

19d0673

fix: code feedback

3ddde18

kth1888 force-pushed the feat/trade_shop branch from 42d4949 to 3ddde18 Compare May 26, 2024 09:52

fix : ui problems at feedback

c78d21a

upa-r-upa requested changes May 27, 2024

View reviewed changes

fix : ui problems #2

4ef0f13

fix : ui problems #3

b9f754a

Atralupus requested changes May 28, 2024

View reviewed changes

upa-r-upa merged commit 4e42bc3 into main May 29, 2024

upa-r-upa deleted the feat/trade_shop branch May 29, 2024 07:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[무역 상점] 인벤토리 및 판매목록 구현 #190

[무역 상점] 인벤토리 및 판매목록 구현 #190

kth1888 commented May 22, 2024 •

edited

Loading

Atralupus May 26, 2024

Atralupus May 26, 2024

Atralupus commented May 26, 2024

Atralupus commented May 26, 2024

upa-r-upa commented May 26, 2024 •

edited

Loading

upa-r-upa left a comment •

edited

Loading

upa-r-upa commented May 27, 2024

Atralupus commented May 28, 2024

upa-r-upa commented May 28, 2024

Atralupus May 28, 2024

Atralupus May 28, 2024

Atralupus May 28, 2024

Atralupus May 28, 2024

Atralupus May 28, 2024

Atralupus May 28, 2024

Atralupus May 28, 2024

Atralupus May 28, 2024

Atralupus May 28, 2024

Atralupus May 28, 2024

		@@ -0,0 +1,3 @@
		[gd_resource type="ButtonGroup" format=3 uid="uid://di66p0jb8cdpk"]

		@@ -0,0 +1,24 @@
		extends Control

		@onready var item_name_label = $Background/MarginContainer/VBoxContainer/NameLabel

[무역 상점] 인벤토리 및 판매목록 구현 #190

[무역 상점] 인벤토리 및 판매목록 구현 #190

Conversation

kth1888 commented May 22, 2024 • edited Loading

TL;DR

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Atralupus commented May 26, 2024

Atralupus commented May 26, 2024

upa-r-upa commented May 26, 2024 • edited Loading

upa-r-upa left a comment • edited Loading

Choose a reason for hiding this comment

upa-r-upa commented May 27, 2024

Atralupus commented May 28, 2024

upa-r-upa commented May 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kth1888 commented May 22, 2024 •

edited

Loading

upa-r-upa commented May 26, 2024 •

edited

Loading

upa-r-upa left a comment •

edited

Loading