Skip to content
This repository has been archived by the owner on Aug 9, 2023. It is now read-only.

feat: not finish #90

Merged
merged 10 commits into from
Aug 3, 2023
Merged

Conversation

crimsonf09
Copy link
Contributor

Change made

  • Bug fixes
  • New features
  • Breaking changes

Describe what you have done

New Features

Fix

Others

@vercel
Copy link

vercel bot commented Aug 3, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
rpkm66-frontend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 3, 2023 11:10pm

Copy link

@khajornritdacha khajornritdacha left a comment

Choose a reason for hiding this comment

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

Feel free to discuss 😄

Comment on lines +7 to +16
<GameContainer
scene={{
id: page,
bg: GameScene[page].bg,
choices: GameScene[page].choices,
message: GameScene[page].message,
goto: GameScene[page].goto,
}}
setPage={setPage}
/>;

Choose a reason for hiding this comment

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

Is it unused? If yes this should be removed

import GameScene from './lib/Scene';

const GameLogic = () => {
const [page, setPage] = useState<string>('Game01');

Choose a reason for hiding this comment

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

"Game01" should be changed into Enum or any const vars instead to prevent typos.

import Image from 'next/image';
import scene4 from '@/public/images/gameBG/scene4.svg';
import scene7 from '@/public/images/gameBG/scene17.svg';
export default function GameBackground(bg: any) {

Choose a reason for hiding this comment

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

Suggested change
export default function GameBackground(bg: any) {
export default function GameBackground({ bg }: { bg: string }) {

Try avoiding any type.

Comment on lines 67 to 73
scene: {
bg: string;
id: string;
message: any;
choices: Question[];
goto: string;
};

Choose a reason for hiding this comment

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

Suggested change
scene: {
bg: string;
id: string;
message: any;
choices: Question[];
goto: string;
};
scene: Scene;

Import type Scene

import scene4 from '@/public/images/gameBG/scene4.svg';
import scene7 from '@/public/images/gameBG/scene17.svg';
export default function GameBackground(bg: any) {
switch (bg.bg) {

Choose a reason for hiding this comment

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

Suggested change
switch (bg.bg) {
switch (bg) {

I am not sure about this but bg is already a type string, isn't?

Comment on lines 5 to 8
export type Scene = Record<
string,
{ bg: string; message: any; choices: Question[]; goto: string }
>;

Choose a reason for hiding this comment

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

Add type scene

Suggested change
export type Scene = Record<
string,
{ bg: string; message: any; choices: Question[]; goto: string }
>;
import React from 'react';
export type Scene = {
id?: string;
bg: string;
message: React.JSX.Element;
choices: Question[];
goto: string;
};
export type Scenes = Record<string, Scene>;

string,
{ bg: string; message: any; choices: Question[]; goto: string }
>;
const GameScene: Scene = {

Choose a reason for hiding this comment

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

Suggested change
const GameScene: Scene = {
const GameScene: Scenes = {

@@ -0,0 +1,127 @@
import type { NextPage } from 'next';

Choose a reason for hiding this comment

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

Move this file to /dto instead as not exporting React Component (e.g. <p>hello</p>) as default will raise errors.

@isd-team-sgcu isd-team-sgcu merged commit db43a70 into dev Aug 3, 2023
2 checks passed
@isd-team-sgcu isd-team-sgcu deleted the feature/rkm66-84-game-before-e-ticket branch August 3, 2023 23:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants