enhancement: fixed typing and separted code

esphung commented 10 months ago

📯 Description

Move to TypeScript ts(x) files so types, classes and code are up-to-date

📝 Summary of changes

src folder contents converted to ts (or tsx) files depending on what they return
standard modern RN boilerplate files added
added TS config files for handling editor configurations
*.d.ts file was stripped down an the appropriate types/interfaces sent to there correct components

🧪 How Has This Been Tested?

[x] iOS builds and runs
[x] Android builds and runs

bberak commented 10 months ago

Thanks for your PR @esphung

I've added a couple of comments..

Just wondering if we need to build anything now (since we've moved the files over to TS) before publishing to NPM? Or is that unnecessary?

Also wondering if we should bump the major version on the package? I suppose you didn't make any breaking API changes -- so that might be overkill..

esphung commented 10 months ago

I was going to suggest adding an example app like what most standard RN libraries come with nowawdays; any create-react-native-library project. I wasn't sure how you would feel about that so I left it out. Considering I have no idea what any of the previous code was doing, I think it would be wise for us to test it for a bit first. I am using the same code for a personal project, so I will be testing a few times a month. Go ahead and leave comments and I'll fix/address them to learn more about the package and we we feel comfortable about it, we can release it. It's a small package, but I don't develop a lot of video games so I'm not a pro at building engines. I do, however, write native bridges between iOS and JS for a living so I'm confident this code will work for the most part. Syntactically, it is correct, but, for example, I left 'Entities' without a type because I wasn't sure if that was intentional. When/if you do end up shipping it,

I suggest we test until the end of the month and then release, after that, I can continue working on it and just make small commits.

PS: I can also add some unit test and GH actions. They are free for public repos

bberak commented 10 months ago

Okey @esphung that's a good idea.. Let's test this PR/branch for a month (I'd like to test it with the latest version of Expo on iOS and Android) before merging into master and publishing a new version on NPM.

I was going to suggest adding an example app like what most standard RN libraries come with nowawdays;

There's an example app that accompanies this library where we can add examples etc: https://github.com/bberak/react-native-game-engine-handbook

I kept it separate to the core library because I didn't want to bloat the core engine with assets and other game-related resources

Syntactically, it is correct, but, for example, I left 'Entities' without a type because I wasn't sure if that was intentional.

Because this was originally a pure-JS library, we bolted on types after (and we kept some of the entity-related types quite loose, because they kind of are 🤷‍♂️)..

PS: I can also add some unit test and GH actions. They are free for public repos

Happy to hear more about this and how it would work ☺️

bberak commented 9 months ago

Hey @esphung

I was testing out this PR today on a demo app, and I found the TS version to be very choppy in terms of registering touch events.. It's weird, the framerate seems good, but it's almost like the touch events are not being registered in a smooth stream.. It's hard to explain, but I've created a branch that replicates this issue:

https://github.com/bberak/react-native-game-engine-handbook (Expo SDK 50; JS version of RNGE; smooth)
https://github.com/bberak/react-native-game-engine-handbook/tree/ts-test (Expo SDK 50; TS version of RNGE; choppy)

If you run the Touch Events > Multi-Touch scene, you'll see what I mean. I can replicate the issue on both iOS and Android. I'm using real devices... I wonder if it's something to do with the changes to the moveThreshold parameter..

I tried to get some screen recordings of the issue:

https://github.com/bberak/react-native-game-engine/assets/1929971/41d8d6a7-3c8e-4cf6-8aee-0b9924432140

https://github.com/bberak/react-native-game-engine/assets/1929971/f152cbf7-5d8f-47d6-b70a-7ff990479765

esphung commented 1 month ago

I'm going to close this PR and possibly try again from the ground up. I will make small PRs to a "mother" branch and we will do this thing. Last time (with this PR), I tried to do too much at once

bberak commented 3 weeks ago

Hey @esphung

Thanks for picking this up again.. Happy to talk any of this through with you.. I feel like this PR was definitely a step in the right direction -- so maybe just some tweaks here and there would've fixed the perf issues I mentioned.. but yeah, totally understand your approach of incremental changes, maybe the best place to start would be to keep the existing functions and classes and add some basic types inline with the source code (rather than in a separate d.ts file). Anyhow, thanks again for pushing this forward 🙏

bberak / react-native-game-engine