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

fix: getSceneName use scenes #3022 #3023

Merged
merged 13 commits into from
Apr 27, 2024

Conversation

aruru-weed
Copy link
Contributor

===:clipboard: PR Checklist :clipboard:===

  • 📌 issue exists in github for these changes
  • 🔬 existing tests still pass
  • 🙈 code conforms to the style guide
  • 📐 new tests written and passing / old tests updated with new scenario(s)
  • 📄 changelog entry added (or not needed)

==================

Closes #3022

Changes:

  • Fixed getSceneName to use scene.

I didn't know how to use _sceneToInstance cache, so I replaced it with something simpler.

@aruru-weed
Copy link
Contributor Author

I committed with the wrong account. ; ;

@eonarheim
Copy link
Member

@aruru-weed No worries, I've done that before!

eonarheim and others added 12 commits April 18, 2024 08:54
…rjs#3027)

Had a conversation here https://discord.com/channels/1195771303215513671/1232877562804305920

```
From example in https://excaliburjs.com/docs/entities:
const entityWithName = new ex.Entity({name: 'Named Entity'});


No overload matches this call.
  Overload 1 of 2, '(options: EntityOptions<any>): Entity<any>', gave the following error.
    Argument of type '{ name: string; }' is not assignable to parameter of type 'EntityOptions<any>'.
      Property 'components' is missing in type '{ name: string; }' but required in type 'EntityOptions<any>'.
  Overload 2 of 2, '(components?: any[] | undefined, name?: string | undefined): Entity<any>', gave the following error.
    Object literal may only specify known properties, and 'name' does not exist in type 'any[]'.ts(2769)
Entity.d.ts(58, 5): 'components' is declared here.


It seems that components is not optional here

export interface EntityOptions<TComponents extends Component> {
    name?: string;
    components: TComponents[];
}

```

Changes:
- Make `components` options optional here.
…balPos getter (excaliburjs#3017)


Closes excaliburjs#2922

## Changes:

- `actor.oldGlobalPos` returns the globalPosition from the previous frame
- `actor.getGlobalPos()` - use `actor.globalPos` instead
- `actor.getGlobalRotation()` - use `actor.globalRotation` instead
- `actor.getGlobalScale()` - use `actor.globalScale` instead

This required some changes to `MotionSystem` so that `globalPos` could be properly captured. Previously, the parent's transform may have already been updated in the system's update when processing a child entity, leading to an incorrect `oldGlobalPos` value. Now, the MotionSystem update captures the entity & all of its children at the same time, skipping any entities that have parents as to not capture them twice over.

This also deprecates other `getGlobalXYZ()` functions and replaces them with getters for consistency.
@eonarheim eonarheim merged commit 8e5f010 into excaliburjs:main Apr 27, 2024
3 checks passed
@eonarheim
Copy link
Member

@aruru-weed Thanks for the contribution! Hope you didn't mind me tossing a couple commits in there!

@aruru-weed aruru-weed deleted the fix-get-scene-name branch April 29, 2024 04:32
eonarheim pushed a commit that referenced this pull request May 5, 2024
Closes #3022

## Changes:

- Fixed `getSceneName` to use `scene`.

I didn't know how to use `_sceneToInstance` cache, so I replaced it with something simpler.
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.

Passing sceneInstance to "Engine.director.getSceneName" does not work it.
5 participants