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

support @base/global module #16247

Merged
merged 10 commits into from
Sep 20, 2023
Merged

support @base/global module #16247

merged 10 commits into from
Sep 20, 2023

Conversation

PPpro
Copy link
Contributor

@PPpro PPpro commented Sep 12, 2023

Re:
#14293
#12647

Changelog

  • remove the internal legacyCC
  • the only public interface is cclegacy
  • support interactive command utils node ./scripts/recast-import.js, this tools gives an interactive way to help fix the import statement, like this
    image
import { js } from './core';
// should change to 
import { js } from '@cocos/core' 
  • setup a @base/global module to encapsure the global-export.ts in core module

What is @base ? What is @pal ?

the engine layer structure should be like this

I think we need another layer called base, which should includes the basic value type like Vec2 Vec3 Size Color Touch, and basic infrastructure like the cclegacy ccwindow global variables.

before we provide this module

our implementation is wierd for now, because our core layer includes the base module,
so here we generated a circular reference like core -> pal -> base -> core
image

after that

so here I wanna create a new layer called base, after that the dependency graph should be like this
image

and that's where the module names come from

@base/global @pal/env @pal/utils means which layer we should put the modules in.
and maybe we could have the layers in the future below:

  • @base/xxx: includes some basic value types and infrastructure, provided to the upper layer
  • @pal/xxx
  • @core/xxx
  • @resources/xxx
  • @cocos/xxx: the function layer of cocos engine, or we call it the framework layer
  • @editor/xxx: to provide some editor related features (I think this should be our final target to develop editor with cocos engine)

How to merge modules

Later if we need to merge all the @pal/xxx into one modules named pal, all we need to do is

  • put all the modules together
  • setup an index file for this module
  • replace and merge all the import statement in engine repo, like
import { touchInputSource } from '@pal/input';
import { findCanvas } from '@pal/env';
// should change to 
import { touchInputSource, findCanvas } from 'pal';

// maybe we could support a tool to do this

We also need to take care whether the module is public, if it's a public module, we need to do some migration after we change the module dependency graph, and here is another task to support the migration mechanism https://github.com/cocos/3d-tasks/issues/17452


Continuous Integration

This pull request:

  • needs automatic test cases check.

    Manual trigger with @cocos-robot run test cases afterward.

  • does not change any runtime related code or build configuration

    If any reviewer thinks the CI checks are needed, please uncheck this option, then close and reopen the issue.


Compatibility Check

This pull request:

  • changes public API, and have ensured backward compatibility with deprecated features.
  • affects platform compatibility, e.g. system version, browser version, platform sdk version, platform toolchain, language version, hardware compatibility etc.
  • affects file structure of the build package or build configuration which requires user project upgrade.
  • introduces breaking changes, please list all changes, affected features and the scope of violation.

@PPpro PPpro marked this pull request as ready for review September 12, 2023 09:58
@PPpro PPpro changed the title remove legacyCC remove legacyCC, only use cclegacy exported from core/index Sep 12, 2023
@github-actions
Copy link

github-actions bot commented Sep 12, 2023

Interface Check Report

! WARNING this pull request has changed these public interfaces:

@@ -40053,9 +40053,9 @@
              * @default dragonBones.AnimationFadeOutMode.All
              * @version DragonBones 5.0
              * @language zh_CN
              */
-            fadeOutMode: AnimationFadeOutMode;
+            fadeOutMode: __private.____node_modules_cocos_dragonbones_js_out_dragonBones__AnimationFadeOutMode;
             /**
              * @private
              */
             fadeOutTweenType: TweenType;
@@ -42313,9 +42313,9 @@
              * </pre>
              * @version DragonBones 4.5
              * @language zh_CN
              */
-            fadeIn(animationName: string, fadeInTime?: number, playTimes?: number, layer?: number, group?: string | null, fadeOutMode?: AnimationFadeOutMode): AnimationState | null;
+            fadeIn(animationName: string, fadeInTime?: number, playTimes?: number, layer?: number, group?: string | null, fadeOutMode?: __private.____node_modules_cocos_dragonbones_js_out_dragonBones__AnimationFadeOutMode): AnimationState | null;
             /**
              * - Play a specific animation from the specific time.
              * @param animationName - The name of animation data.
              * @param time - The start time point of playing. (In seconds)
@@ -42570,9 +42570,9 @@
              * - 已废弃,请参考 {@link #play()} {@link #fadeIn()}。
              * @deprecated
              * @language zh_CN
              */
-            gotoAndPlay(animationName: string, fadeInTime?: number, duration?: number, playTimes?: number, layer?: number, group?: string | null, fadeOutMode?: AnimationFadeOutMode, pauseFadeOut?: boolean, pauseFadeIn?: boolean): AnimationState | null;
+            gotoAndPlay(animationName: string, fadeInTime?: number, duration?: number, playTimes?: number, layer?: number, group?: string | null, fadeOutMode?: __private.____node_modules_cocos_dragonbones_js_out_dragonBones__AnimationFadeOutMode, pauseFadeOut?: boolean, pauseFadeIn?: boolean): AnimationState | null;
             /**
              * - Deprecated, please refer to {@link #gotoAndStopByTime()}.
              * @deprecated
              * @language en_US
@@ -68019,8 +68019,74 @@
         }
         export interface _cocos_dragon_bones_ArmatureDisplay__BoneIndex extends Number {
             _any: number;
         }
+        /**
+         * - Animation fade out mode.
+         * @version DragonBones 4.5
+         * @language en_US
+         */
+        /**
+         * - 动画淡出模式。
+         * @version DragonBones 4.5
+         * @language zh_CN
+         */
+        export enum ____node_modules_cocos_dragonbones_js_out_dragonBones__AnimationFadeOutMode {
+            /**
+             * - Do not fade out of any animation states.
+             * @language en_US
+             */
+            /**
+             * - 不淡出任何的动画状态。
+             * @language zh_CN
+             */
+            None = 0,
+            /**
+             * - Fade out the animation states of the same layer.
+             * @language en_US
+             */
+            /**
+             * - 淡出同层的动画状态。
+             * @language zh_CN
+             */
+            SameLayer = 1,
+            /**
+             * - Fade out the animation states of the same group.
+             * @language en_US
+             */
+            /**
+             * - 淡出同组的动画状态。
+             * @language zh_CN
+             */
+            SameGroup = 2,
+            /**
+             * - Fade out the animation states of the same layer and group.
+             * @language en_US
+             */
+            /**
+             * - 淡出同层并且同组的动画状态。
+             * @language zh_CN
+             */
+            SameLayerAndGroup = 3,
+            /**
+             * - Fade out of all animation states.
+             * @language en_US
+             */
+            /**
+             * - 淡出所有的动画状态。
+             * @language zh_CN
+             */
+            All = 4,
+            /**
+             * - Does not replace the animation state with the same name.
+             * @language en_US
+             */
+            /**
+             * - 不替换同名的动画状态。
+             * @language zh_CN
+             */
+            Single = 5
+        }
         export interface ____node_modules_typescript_lib_libdom__EXT_color_buffer_half_float {
             readonly FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE_EXT: GLenum;
             readonly RGB16F_EXT: GLenum;
             readonly RGBA16F_EXT: GLenum;

@PPpro PPpro marked this pull request as draft September 13, 2023 03:50
update

add

update

updatte
@minggo
Copy link
Contributor

minggo commented Sep 13, 2023

Yep, i like the idea to have base module. But from the codes above, it seems there are more modules in base space. Can i know what modules will be included in base space? Currently, i saw @base/global, any other modules?

@PPpro PPpro changed the title remove legacyCC, only use cclegacy exported from core/index support @base/global module Sep 13, 2023
@PPpro
Copy link
Contributor Author

PPpro commented Sep 13, 2023

Yep, i like the idea to have base module. But from the codes above, it seems there are more modules in base space. Can i know what modules will be included in base space? Currently, i saw @base/global, any other modules?

for now I can say, it should also includes

  • @base/debug: provide some log utils and debug info
  • @base/js: our js utils
  • @base/event: EventTarget is all the layers need to emit event
  • @base/value-types: includes
    • some math types like Vec2 Vec3 Mat3 ...
    • Size Color Rect
    • Touch, and maybe Event?

@minggo

@PPpro PPpro marked this pull request as ready for review September 13, 2023 07:46
@PPpro PPpro marked this pull request as draft September 13, 2023 07:59
import { getUuidFromURL, transform } from './helper';
import parser, { Parser } from './parser';
import parser from './parser';
import { Parser } from './parser';
Copy link
Contributor

Choose a reason for hiding this comment

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

Can just write like

import { parser, Parser } from './parser';

?

Copy link
Contributor Author

@PPpro PPpro Sep 13, 2023

Choose a reason for hiding this comment

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

yes, the recast-import tools auto separates the default import and named import, I'll fix this mannually

Copy link
Contributor Author

Choose a reason for hiding this comment

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

import Cache from './cache';
import downloadFile, { FileProgressCallback } from './download-file';
import downloadFile from './download-file';
import { FileProgressCallback } from './download-file';
Copy link
Contributor

Choose a reason for hiding this comment

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

The same as above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

There are many codes is modified like this. I don't mark all these codes.

Copy link
Contributor Author

@PPpro PPpro Sep 13, 2023

Choose a reason for hiding this comment

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

I am gonna keep this two separate import statement and revert the fix here 701ccf5

because:

  1. I think it's not a good implementation to merge default import and named import together.
import downloadFile from './download-file';  // this is default import
import { FileProgressCallback } from './download-file';  // this is named import

TS also support type import, try to consider this situation

import type downloadFile from './download-file';
import type { FileProgressCallback } from './download-file';

// there is no way to merge this two import statement
// TS syntax doesn't support to merge like this:
import type downloadFile, type { FileProgressCallback } from './download-file';
// also TS syntax doesn't support:
import type downloadFile, { FileProgressCallback } from './download-file';
import downloadFile, type { FileProgressCallback } from './download-file';

so I think we should take a unified approach to separate the default import and named import:

// value import 
import downloadFile from './download-file';
import { FileProgressCallback } from './download-file';
// type import 
import type downloadFile from './download-file';
import type { FileProgressCallback } from './download-file';
  1. another reason is that for the implementation of recast-import tool, we've tried to merge the same import statement:
import { Vec2 } from './core/math';
import { cclegacy } from './core/index';
// should be merged to
import { Vec2, cclegacy  } from './core/index';

but for default import statement, we cannot merge them, try to consider this situation:

// we don't know how to merge this three import statement
import defaultName1 from './core/index';
import defaultName2 from './core/index';
import { cclegacy }from './core/index';

// TS syntax doesn't support:
import defaultName1, defaultName2, { cclegacy  } from './core/index';

So I think it's better that we don't try merging default import and named import, and keep them separated instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

So should we use default export?

Copy link
Contributor Author

@PPpro PPpro Sep 15, 2023

Choose a reason for hiding this comment

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

yes we can use, it's actually a syntax sugar

import defaultName from './core/index';
// equals to 
import { default as defaultName } from './core/index';

it's better not to use default export

This reverts commit 747bea8.
@PPpro PPpro marked this pull request as ready for review September 19, 2023 06:13
@minggo minggo merged commit 86399f2 into cocos:develop Sep 20, 2023
9 checks passed
cocos-robot pushed a commit to cocos-robot/engine that referenced this pull request Sep 20, 2023
@PPpro PPpro deleted the dev-legacyCC branch September 20, 2023 02:39
minggo pushed a commit that referenced this pull request Sep 20, 2023
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.

2 participants