Skip to content

Conversation

@hsw1920
Copy link
Collaborator

@hsw1920 hsw1920 commented Nov 24, 2024

커밋단위로 리뷰하는게 가장 편하십니다.

  • 작업이 복잡하여 잘 분리하여 커밋해두었습니다.

🤔 배경

  • 스티커뷰에 닉네임을 표시하거나 owner를 표기하기 위해 커스텀 UIImageView가 필요했습니다.
  • 해당 스티커뷰는 탭 이벤트를 제공해야했습니다.
  • EventHub를 거쳐 Host, Guest간 통신에 문제가 없었어야 했습니다.

📃 작업 내역

  • StickerView 구현: StickerView UItapPublisher를 통한 탭 이벤트 처리 구현
  • StickerView 업데이트 최적화: StickerEntity 상태 비교를 통해 불필요한 업데이트 방지
  • 스티커 선택 비즈니스 로직 구현: StickerViewtapunlock/lockEventHub 반영 처리.
  • StickerEntity 배열 확장: owner 확인, unlock/lock 로직 재사용을 위한 유틸리티 메서드 추가
  • Host/Guest 구분 기능 추가: 임시적으로 사용자 역할을 구분합니다.
    • private let owner = "GUEST" + UUID().uuidString.prefix(4)
    • 임시로 UUID를 이용해 각 Guest들의 owner를 유니크하게 구분할 수 있습니다.

✅ 리뷰 노트

StickerView에서 tap관련 cancellable 관리

// StickerView.swift
    func tapHandler(_ completion: @escaping (UUID) -> Void) {
        tapPublisher
            .sink { [weak self] _ in
                guard let self else { return }
                completion(sticker.id)
            }
            .store(in: &cancellables)
    }

// EditPhotoRoomViewController.swift
    private func addNewSticker(to sticker: StickerEntity, isLocal: Bool) {
        registerSticker(for: sticker)
        
        let stickerView = StickerView(sticker: sticker)
        stickerView.tapHandler { [weak self] stickerID in
            self?.input.send(.stickerViewDidTap(stickerID))
        }
        
        canvasScrollView.imageView.addSubview(stickerView)
        stickerView.update(with: sticker)
        input.send(.createSticker(sticker))
    }

가장 큰 이유는 StickerView가 자주 생성되고 삭제된다는 점에 있었습니다.

스티커는 동적으로 생성/삭제 되므로 ViewController가 개별 스티커의 이벤트 구독을 관리하기에 불편했습니다.

스티커가 삭제될 때 구독을 해제해야 하며, 이를 ViewController에서 수동으로 추적하여 처리하기에는 오버헤드가 크다고 판단했습니다.

따라서 tapHandler(_:) 탈출 클로저를 제공하여 구독은 StickerView 내부에서 관리하고, StickerView가 삭제되면 내부적으로

cancellable이 해제되어 메모리 누수를 방지하고자 이 방식을 채택했습니다.

같은 값일 때 불필요한 할당 방지

    private func updateFrame(to frame: CGRect) {
        guard sticker.frame != frame else { return }
        
        sticker.updateFrame(to: frame)
        self.frame = frame
    }
    
    private func updateOwner(to owner: String?) {
        guard sticker.owner != owner else { return }
        
        sticker.updateOwner(to: owner)
        if let owner = owner {
            nicknameLabel.text = owner
            layer.borderWidth = 1
        } else {
            nicknameLabel.text = nil
            layer.borderWidth = 0
        }
    }
  • 같은 값을 재할당하는 것으로 불필요한 오버헤드를 발생시킬 수 있을 것 같아 변경여부를 확인한 뒤 할당하였습니다.
  • StickerView또한 ViewModel을 두어 removeDuplicate()를 통해 관리할 수도 있었지만,
  1. 추가적인 ViewModel 구현을 지양하고 struct를 고수하고자
  2. 추가적인 StickerEntity를 관리하는 DataSource를 두는 것을 지양하고자
    이 방식을 택하였습니다.

SitckerView 탭 이벤트 처리 비즈니스 로직 구현

    private func handleStickerViewDidTap(with stickerID: UUID) {
        // MARK: 선택할 수 있는 객체인지 확인함
        guard canInteractWithSticker(id: stickerID) else { return }
        
        // MARK: 필요시 이전 스티커를 unlock하고 반영함
        unlockPreviousSticker()
        
        // MARK: Tap한 스티커를 lock하고 반영한다.
        lockTappedSticker(id: stickerID)
    }

선택 가능한 객체 확인 (canInteractWithSticker)

  • StickerView 탭 이벤트 발생 시, 해당 스티커가 선택 가능한 상태인지(소유 여부 확인) 판단하는 로직 추가.
  • stickerList.isOwned 메서드 활용으로 코드를 간결하게 구성했습니다.

이전 선택된 스티커 해제 (unlockPreviousSticker)

  • 기존에 선택된 스티커를 해제하고, EventHub에 해제 상태를 반영하도록 처리.
  • 스티커 해제는 lockedSticker와 unlock 메서드를 활용해 구현.

현재 선택된 스티커 잠금 (lockTappedSticker)

  • 선택된 스티커를 잠금 상태로 설정하고, EventHub에 업데이트 요청.
  • stickerList.lock 메서드를 사용해 간결하게 구현.

상태 관리 최적화

  • stickerObjectListSubject.value를 활용해 상태를 읽고 업데이트하며, 변경 시 필요한 부분만 EventHub와 동기화.

🎨 스크린샷

dd

  • 탭한 이벤트가 전파됩니다.
  • 소유하지 못한 객체는 비즈니스 로직에서 이벤트를 전파하지 못하도록 걸러집니다.

🚀 테스트 방법

  1. EditPhotoRoomFeatureDemo에서 Host, Guest로 두 개의 디바이스로 런
  2. 임시: [프레임] 버튼을 눌러 커넥션 연결
  3. 스티커 버튼을 생성하고 tap을 통해 테스트가 가능합니다.

- tapPublisher에 대한 구독은 StickerView에서 관리할 수 있도록 탈출 클로저 구현
- StickerView는 쉽게 생성/삭제가 가능하므로 EditPhotoViewController에서 cancellables에서 관리하기에는 무리가 있었음.
- StickerEntity는 struct이므로 변경을 위해 mutating 메서드 구현
- StickerEntity는 매우 자주 참조 되어 변경될 수 있어 class로 두는 것을 고려했지만, 그만큼 상태관리에 매우 큰 어려움이 있기 때문에 불변성을 위해 struct를 유지하였음.
- StickerView를 업데이트 하는 것을 ViewController가 직접 변경하는 것이 아닌 StickerView에게 요청하도록 책임을 분리
- StickerView는 이전 StickerEntity를 통해 이전 상태를 파악하여 변경이 없다면 업데이트하지 않도록 구현
  - StickerView에 StickerEntity를 두어 메모리는 추가되었지만, 이전 상태를 알고 불필요한 업데이트를 하지 않는 것이 더 리소스가 적다고 판단하였음.
  - 또한 StickerEntity 하나의 데이터는 상대적으로 매우 작기 때문에 이러한 방식이 적합하다고 판단함.
- SticekrView를 탭하는 이벤트 추가
- unlock을 위해서는 EventHub에서 해당 Sticker의 unlock을 요청하는 owner와 기존 owner를 비교할 수 있어야했으나 이 기능이 없었음.
- 기존 코드는 EventHub로부터 stickerList를 받았을 때 변경이 없는 경우 무시하는 코드였음
- 하지만 이를 비교하는 로직은 StickerEntity의 id만으로 같은 지 확인하기 때문에 예상한 동작이 일어나지 않았음.
- StickerView 단에서 변경사항을 파악하여 업데이트 여부를 결정하기 때문에 제거하였음.
- 임시 값으로써 추후 ConnectionClient에서 User를 구분하는 기능이 추가될 시 제거 예정
### Flow
1. 선택할 수 있는 객체인지 확인함 (owner가 nil이거나, 자신이거나)
2. 자신이 선택하는 객체는 유일함. 따라서 기존에 선택했던 객체가 있다면 unlock을 하고 이를 EventHub에 반영을 요청.
3. 선택한 객체를 lock하고 EventHub에 반영을 요청

### Extension
- 코드 중복을 방지하기 위해 자주 사용하는 StickerEntity 배열을 확장하였음.
- `isOwned(id: UUID, owner: String) -> Bool` : 해당 `id`와 `owner`를 통해 해당 `id`의 `StickerEntity`가 내가 소유할 수 있는 지 확인하는 메서드
- `lockedSticker(by owner: String) -> StickerEntity?` : 자신이 소유하고 있는 `StickerEntity`를 반환하는 메서드
- `unlock(by owner: String)` : 내가 소유하고 있는 `StickerEntity`를 unlock하는 메서드
- `lock(by id: UUID, owner: String) -> StickerEntity?` : 해당 `id`의 `StickerEntity`를 자신이 소유하도록 요청하는 메서드
lock을 확인하거나, lock을 하거나, 선택가능한 지 확인 하는 것은 모두 재활용이 가능함
- queue가 비었을 때는 popPublisher가 false를 방출해야함.
- queue가 보낼 게 있을 때만 true여야 함.
@hsw1920 hsw1920 self-assigned this Nov 24, 2024
@hsw1920 hsw1920 linked an issue Nov 24, 2024 that may be closed by this pull request
3 tasks
@hsw1920 hsw1920 requested review from 0Hooni, Kiyoung-Kim-57 and youn9k and removed request for 0Hooni and Kiyoung-Kim-57 November 24, 2024 17:13
@hsw1920 hsw1920 added ✨ feat 새로운 기능 추가 ⚙️ refactor 코드 정상화 labels Nov 24, 2024
Copy link
Member

@Kiyoung-Kim-57 Kiyoung-Kim-57 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

우와...설계부터 구현까지 엄청 복잡하네요!! 고생하셨습니다!

- Combine 제거 및 Delegate로 구현
- StickerView에서 불필요하게 Subject 인스턴스를 가지지 않아도 됨
Copy link
Collaborator

@0Hooni 0Hooni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 💯

수고 많으셨습니다!

Copy link
Member

@youn9k youn9k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다! 손이 빠른 구현 고수..

@hsw1920 hsw1920 merged commit ff54422 into develop Nov 25, 2024
1 check passed
@hsw1920 hsw1920 deleted the feat/#77-select-sticker branch November 25, 2024 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

✨ feat 새로운 기능 추가 ⚙️ refactor 코드 정상화

Projects

None yet

Development

Successfully merging this pull request may close these issues.

스티커를 선택할 수 있다

5 participants