Skip to content
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

feat: ASMusicKit 프레임워크로 분리 #25

Merged
merged 9 commits into from
Nov 13, 2024
Merged

feat: ASMusicKit 프레임워크로 분리 #25

merged 9 commits into from
Nov 13, 2024

Conversation

moral-life
Copy link
Collaborator

What is this PR?

  • PR에 대한 설명

ASMusicKit 프레임워크로 분리.
MusicKit을 활용해 게임에 쓰일 데이터에 맞게 가공하여 가져오는 프레임워크

ASMusicKit

  • ASMusicAPI
    • search 메서드

PR Type

  • Bugfix
  • Chore
  • New feature (기능을 추가하는 feat)
  • Breaking change (기존의 기능이 동작하지 않을 수 있는 fix/feat)
  • Documentation Update

Further comments

실행과정중에 public을 사용하는 프레임워크 코드와 Sendable과 관련한 문제가 발생하였고 이부분은 엔지니어링 위키에 기술해놓음

1. search를 Songs를 받아오는것, Song을 ASSong으로 변환하는 두가지 작업을 하는데 이를 분리하는 것이 좋을지
2. 위 방식을 택했을 때 MusicKit 예시 앱 처럼 파도타기 (노래 -> 관련 앨범 -> 아티스트 -> 다른앨범 -> ....) 하는 기능을 잃게 될 것 같은데 괜찮은지
실제 MusicKit에서 Artwork를 관리하는 방법은 다른 것 같아 이부분 논의 필요
Font, Media Privacy 관련한 row가 conflict 발생하여 수동으로 resolve 해주었는데 이 과정에서 plist syntax를 어긋나는 문제가 있었음. 다시 수정 하여 현재는 해결함
@moral-life moral-life added the feat 기능 구현 label Nov 12, 2024
@moral-life moral-life self-assigned this Nov 12, 2024
Copy link
Collaborator

@Tltlbo Tltlbo left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!

/// search 결과를 Assong으로 변환 하여 배열로 리턴하는 함수
/// 노래 검색화면에 기본으로 뜨는 row 개수가 6개이므로 별다른 지정 없으면 기본값 6으로 지정
func search(for text: String, _ maxCount: Int = 6) async -> [ASSong] {

Copy link
Collaborator

Choose a reason for hiding this comment

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

P3: 함수의 첫시작은 붙여서 작성!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

해당파일은 adcbcdc 커밋에서 삭제됐습니다! public 붙이고 린트 수정해서 프레임워크로 옮겨놨어용!

Copy link
Collaborator

@psangwon62 psangwon62 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!

Comment on lines 6 to 11
/// MusicKit을 통해 AppleMusic
/// - Parameters:
/// - text: 검색 요청을 보낼 검색어
/// - maxCount: 검색해서 찾아올 음악의 갯수 기본값 설정은 25
/// - Returns: ASSong의 배열
public func search(for text: String, _ maxCount: Int = 25) async -> [ASSong] {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Description 좋네요!!! 👍

Comment on lines 13 to 34
switch status {
case .authorized:
do {
var request = MusicCatalogSearchRequest(term: text, types: [Song.self])
request.limit = maxCount
request.offset = 1

let result = try await request.response()
let asSongs = result.songs.map { song in
let artworkURL = song.artwork?.url(width: 100, height: 100)
return ASSong(title: song.title, artistName: song.artistName, artwork: artworkURL)
}
return asSongs
} catch {
print(String(describing: error))
return []
}
default:
print("Not authorized to access Apple Music")
return []
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3; switch 문의 case에 인덴트 적용해 주세용

/// - text: 검색 요청을 보낼 검색어
/// - maxCount: 검색해서 찾아올 음악의 갯수 기본값 설정은 25
/// - Returns: ASSong의 배열
public func search(for text: String, _ maxCount: Int = 25) async -> [ASSong] {
Copy link
Collaborator

Choose a reason for hiding this comment

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

p4;
offset도 받는게 나중에 스크롤 구현할떄 좋을거같아요 .!

ASMusicKit/ASMusicAPI.swift Outdated Show resolved Hide resolved
@moral-life moral-life requested a review from a team as a code owner November 13, 2024 06:14
@moral-life moral-life merged commit bc4049c into dev Nov 13, 2024
@moral-life moral-life linked an issue Nov 13, 2024 that may be closed by this pull request
2 tasks
@psangwon62 psangwon62 deleted the feat/#13 branch November 25, 2024 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat 기능 구현
Projects
None yet
Development

Successfully merging this pull request may close these issues.

노래 검색
4 participants