Skip to content

Feature/planet arrange#6

Open
shirokuma222 wants to merge 4 commits intomainfrom
feature/planetArrange
Open

Feature/planet arrange#6
shirokuma222 wants to merge 4 commits intomainfrom
feature/planetArrange

Conversation

@shirokuma222
Copy link

惑星(地球)の配置機能を一旦追加
・levaで座標、大きさ、回転速度を指定して地球を配置できるようにした。
・プレビューを実装。
・クリック配置の機能を実装。
・ついでに一覧表示と削除も追加。
Simulation/index.tsxに全部書いたが、配置機能は別で管理すべきかもしれない。

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Feb 19, 2026

Deploying space-simulator with  Cloudflare Pages  Cloudflare Pages

Latest commit: 3f5cc98
Status: ✅  Deploy successful!
Preview URL: https://ac4d79a1.space-simulator.pages.dev
Branch Preview URL: https://feature-planetarrange.space-simulator.pages.dev

View logs

import type { Planet } from "@/types/planet";
import { earth } from "@/data/planets";

interface PlanetMeshProps {
Copy link
Contributor

Choose a reason for hiding this comment

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

interface と type のどっちを使うかっていう論争があって、まあ正直どっちでもいいんですけど、園部くんの PR の方で type の方が直感的でいいんじゃないかっていう話をしたので、type に統一してほしいです。

);
}

interface PlacementSurfaceProps {
Copy link
Contributor

Choose a reason for hiding this comment

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

ここも type を使ってほしいです。

<pointLight position={[10, 10, 10]} intensity={3} />

{planets.map((planet, index) => (
<PlanetMesh key={`planet-${index}`} planet={planet} />
Copy link
Contributor

Choose a reason for hiding this comment

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

map の key に配列のインデックスを使うのはよくあるアンチパターンなので、planet の型定義に name プロパティを追加してあげて、それをユニークな識別子に使うのがいいと思います。ただ、この話は園部くんの PR の方でもしていてコンフリクトすると思うので、園部くんの PR がマージされてから pull して直すようにしてください。

<ul className="mb-0 mt-2.5 list-none p-0">
{planets.map((planet, index) => (
<li
key={`planet-item-${index}`}
Copy link
Contributor

Choose a reason for hiding this comment

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

ここでも key に配列のインデックスを使ってますね。このまま惑星を削除したり追加したりしたらバグの原因になります。

const [placementMode, setPlacementMode] = useState(false);
const [placementPanelOpen, setPlacementPanelOpen] = useState(true);

const [planetControls, setPlanetControls, getPlanetControl] = useControls("New Planet", () => ({
Copy link
Contributor

Choose a reason for hiding this comment

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

可能ならここの useControls を一つにまとめてほしいです。

Copy link
Contributor

Choose a reason for hiding this comment

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

無理ならそのままで大丈夫です。

);
}

interface PreviewPlanetProps {
Copy link
Contributor

Choose a reason for hiding this comment

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

ここも type を使ってほしいです。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments