bberak / react-native-game-engine

A lightweight Game Engine for React Native 🕹⚡🎮
MIT License
2.87k stars 172 forks source link

Functions not in TypeScript definition files #60

Open SimpleProgrammingAU opened 3 years ago

SimpleProgrammingAU commented 3 years ago

I haven't checked the other classes, but GameEngine.js does not appear to have its functions referenced in react-native-game-enging.d.ts:

    export interface GameEngineProperties {
      systems?: any[];
      entities?: {} | Promise<any>;
      renderer?: any;
      touchProcessor?: any;
      timer?: any;
      running?: boolean;
      onEvent?: any;
      style?: StyleProp<ViewStyle>;
      children?: React.ReactNode;
    }

    export class GameEngine extends React.Component<GameEngineProperties> {}

Should GameEngine not be:

  export class GameEngine extends React.Component<GameEngineProperties> {
      dispatch: (event:any) => void;
      start: () => void;
      stop: () => void;
      swap: ({}:any | Promise) => void | Promise<void>
    }

Or something similar to?

Is this an issue for other classes?

bberak commented 3 years ago

Hi @SimpleProgrammingAU,

Thanks for pointing this out! I'm getting a bit more comfortable with TS now, so should be able to review your PR and get it merged ASAP (it might have to wait till after the festive season unfortunately).

Thanks again for your contribution!

SimpleProgrammingAU commented 3 years ago

On a similar front, I am trying to come up with ways to deal with the GameEngine entities.renderer property. It accepts a React.Component object without any props as its value. This invariably leads to either having to make all of the props for each renderer optional (which means you have to include checks for undefined) or something else? Not actually sure what best-practice would be in a situation like this.

Hopefully yourself or someone who knows TS better than I can provide a good solution.

bberak commented 3 years ago

I see - as in you have written a renderer component and you want to add type safety to the props?

Are you able to use defaultProps or default values to avoid the null checks whilst still keeping the items in your props interfaces required?

Otherwise, I imagine the challenge will be to overcome the dynamic nature of the GameEngine's default renderer. Whilst you can switch out the GameEngine's renderer for something more custom - I'm not quite sure how you would go about that in a type-safe manner.

bberak commented 3 years ago

Also, just wondering where the TS compiler complains?

Would something like this suffice:

<GameEngine entities={{ player: { x: 0, y: 0, renderer: <PlayerRenderer x={0} y={0} />  }}} />

... Or this form, which should also work:

<GameEngine entities={{ player: { x: 0, y: 0, renderer: PlayerRenderer  }}} />
SimpleProgrammingAU commented 3 years ago

I'll try them both! The first one doesn't feel good having the values entered twice but if the second one works, that is ideal!

Make sure to update the typedefs to allow JSX.element and React.Component if either can work =)

SimpleProgrammingAU commented 3 years ago

Using the second option worked perfectly. No issues with the TS-linter.

Thanks heaps for the help! Now I'm just trying to come up with ways to be able to make TS recognise the entities object to have that requirement. I'm gonna have a look and might update my pull request if I work it out.

I tried this but it didn't catch an incorrect value for renderer:

interface GameEngineEntities {
    [key:string | number]: {
      [key:string]: any,
      renderer?: JSX.Element | React.Component,
    }
  }

  export interface GameEngineProperties {
    systems?: any[];
    entities?: GameEngineEntities | Promise<any>;
    renderer?: any;
    touchProcessor?: any;
    timer?: DefaultTimer | any;
    running?: boolean;
    onEvent?: any;
    style?: StyleProp<ViewStyle>;
    children?: React.ReactNode;
  }

Not sure what to try next.

SimpleProgrammingAU commented 3 years ago

I have updated the pull request #61 to include a fix for the renderer property after soliciting some help from Reddit.

bberak commented 3 years ago

Awesome @SimpleProgrammingAU, I took a quick look at the PR and it looks really solid. Huge thanks. I'll be 100% back online next week - I'll merge it in then.