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

Tanks Tutorial Inaccuracies #286

Open
samueldcorbin opened this issue Mar 22, 2024 · 1 comment
Open

Tanks Tutorial Inaccuracies #286

samueldcorbin opened this issue Mar 22, 2024 · 1 comment

Comments

@samueldcorbin
Copy link

samueldcorbin commented Mar 22, 2024

The readme here has many incorrect statements that do not match the code: https://github.com/Unity-Technologies/EntityComponentSystemSamples/blob/master/EntitiesSamples/Assets/Tutorials/Tanks/README.md

This isn't in the readme, but Step 1 links https://github.com/Unity-Technologies/EntityComponentSystemSamples/blob/master/EntitiesSamples/Assets/Tutorials/Tanks/Step%202/TurretRotationSystem.cs which says:

    // Unmanaged systems based on ISystem can be Burst compiled, but this is not yet the default,
    // so we have to explicitly opt into Burst compilation with the [BurstCompile] attribute.
    // It has to be added on BOTH the struct AND the OnCreate/OnDestroy/OnUpdate functions to be
    // effective.

This stresses that the attribute must be on both the struct and the method, but the code doesn't put it on the struct. I believe the comment is the part that is wrong, but I'm not sure.

Step 3 in the readme says: Introduces scheduling a parallel job. but the code in those files does not schedule any job at all.

Step 4 says Introduces aspects, entity prefabs, EntityCommandBuffer, and IJobEntity. but the code does not actually include an IJobEntity or an EntityCommandBuffer (although it probably should since it's performing a structural change!)

The file in step 4 also has an inconsistent comment here: https://github.com/Unity-Technologies/EntityComponentSystemSamples/blob/master/EntitiesSamples/Assets/Tutorials/Tanks/Step%204/TurretAspect.cs#L18 (it talks about making it Optional, but doesn't actually do anything to make it Optional).

Step 5 has several issues. The link to CannonBallAspect.cs is broken. The description says it uses the Aspect, but it actually just uses the Cannonball component directly (despite creating the Aspect in the previous step). The description says Introduces scheduling a parallel job with IJobEntity. but it doesn't actually use ScheduleParallel (it just uses Schedule). Also Step 3 was already supposed to introduce scheduling a parallel job (though it doesn't actually do that).

Step 6 has a mysterious unexplained state.RequireForUpdate<Execute.TankSpawning>(); here that I've never seen before: https://github.com/Unity-Technologies/EntityComponentSystemSamples/blob/master/EntitiesSamples/Assets/Tutorials/Tanks/Step%206/TankSpawningSystem.cs#L16

Step 7 says Introduces enableable components (IEnableableComponent), live conversion, and storing basic configuration data in a singleton. but the config singleton was created in the previous step. The linked file also has state.RequireForUpdate<Execute.SafeZone>(); again, with no explanation again. What is Execute.SafeZone and how is it different from SafeZone?!

Step 8 again has the mysterious state.RequireForUpdate<Execute.Camera>(); in https://github.com/Unity-Technologies/EntityComponentSystemSamples/blob/master/EntitiesSamples/Assets/Tutorials/Tanks/Step%208/CameraSystem.cs which is even more mysterious here because there isn't anything in the file even called Execute.

@samueldcorbin samueldcorbin changed the title Tanks Tutorial Incorrect Readme Tanks Tutorial Inaccuracies Mar 22, 2024
@ConanGentleman
Copy link

I agree with you

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

No branches or pull requests

2 participants