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 hot update issue. #2312

Merged
merged 8 commits into from Apr 22, 2020
Merged

Conversation

xianyinchen
Copy link
Contributor

@xianyinchen xianyinchen commented Mar 19, 2020

  • 添加获取更新包大小接口
  • 提升对 CDN 更新的友好度,对链接添加MD5参数
  • 更新过程中断导致的问题
  • 热更新下载启动时,会立即删除旧版本文件问题
  • 热更新下载完成后,文件复制操作可能被中断,导致更新后资源丢失或混乱(移动到JS层处理)

@holycanvas
Copy link
Contributor

热更新文档里面要更新一下么?

@xianyinchen
Copy link
Contributor Author

xianyinchen commented Mar 20, 2020

原生层的修改不涉及到热更新流程改动,可以兼容旧版本的,这个PR可以保持文档不变。

热更新测试例的改动,需要更新文档
cocos-creator/cocos-tutorial-hot-update#31

@holycanvas holycanvas requested review from PPpro and removed request for PPpro March 20, 2020 08:24
@@ -1215,6 +1232,7 @@ bool js_register_extension_AssetsManagerEx(se::Object* obj)
cls->defineFunction("downloadFailedAssets", _SE(js_extension_AssetsManagerEx_downloadFailedAssets));
cls->defineFunction("isResuming", _SE(js_extension_AssetsManagerEx_isResuming));
cls->defineStaticFunction("create", _SE(js_extension_AssetsManagerEx_create));
cls->defineStaticFunction("checkFinish", _SE(js_extension_AssetsManagerEx_checkFinish));
Copy link
Contributor

Choose a reason for hiding this comment

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

不需要更新自动绑定代码

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这是因为添加了静态函数,可以在引擎启动前,修复热更新中断的问题,比如热更新完成后,从_tempStoragePath拷贝文件到_storagePath的操作被中断,导致_storagePath出现新旧文件混合,这种情况需要在引擎启动前把_tempStoragePath未拷贝的文件移动到_storagePath。

@@ -176,6 +176,79 @@ AssetsManagerEx* AssetsManagerEx::create(const std::string& manifestUrl, const s
return ret;
}

void AssetsManagerEx::checkFinish(const std::string &storagePath) {
AssetsManagerEx* mgr = new (std::nothrow) AssetsManagerEx("", storagePath);
Copy link
Contributor

Choose a reason for hiding this comment

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

这个 API 居然设计为 static ?不是很能理解,这应该是一个成员属性,检查自身状态才对啊

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个接口不是给用户调用的,而是用于在启动引擎前,检查热更新是否存在问题,并修复问题。

// this is the safest.
if (_tempManifest != nullptr
&& _tempManifest->isLoaded()
&& _remoteManifest->versionGreater(_tempManifest, _versionCompareHandle)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

这里似乎不用对比 version,既然已经在 if (_localManifest->versionGreaterOrEquals(_remoteManifest, _versionCompareHandle)) 的 else 中,那么这个 version 判断自然是成立的

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个修复,是为了考虑一种特殊情况:
用户下载A版本未完成,然后发现B版本,开启新的热更新,但是这时候_tempStoragePath里面是有A版本的缓存文件的,这里做了清除的A版本资源的操作。

Copy link
Contributor

Choose a reason for hiding this comment

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

我知道,我的意思是从逻辑上来说

if (_tempManifest != nullptr && _tempManifest->isLoaded())
{
}

就足够了

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我重新确认下代码,加这个代码是多余了的,原始代码下载version.manifest后会下载project.manifest,然后做了类似的操作,我移除添加的这个代码块。

void AssetsManagerEx::prepareUpdate()
{
    if (_updateState != State::NEED_UPDATE)
        return;

    // Clean up before update
    _failedUnits.clear();
    _downloadUnits.clear();
    _totalWaitToDownload = _totalToDownload = 0;
    _nextSavePoint = 0;
    _percent = _percentByFile = _sizeCollected = _totalDownloaded = _totalSize = 0;
    _downloadResumed = false;
    _downloadedSize.clear();
    _totalEnabled = false;

    // Temporary manifest exists, previously updating and equals to the remote version, resuming previous download
    if (_tempManifest && _tempManifest->isLoaded() && _tempManifest->isUpdating() && _tempManifest->versionEquals(_remoteManifest))
    {
        _tempManifest->saveToFile(_tempManifestPath);
        _tempManifest->genResumeAssetsList(&_downloadUnits);
        _totalWaitToDownload = _totalToDownload = (int)_downloadUnits.size();
        _downloadResumed = true;

        // Collect total size
        for(auto iter : _downloadUnits)
        {
            const DownloadUnit& unit = iter.second;
            if (unit.size > 0)
            {
                _totalSize += unit.size;
            }
        }
    }
    else
    {
        // Temporary manifest exists, but can't be parsed or version doesn't equals remote manifest (out of date)
        if (_tempManifest)
        {
            // Remove all temp files
            _fileUtils->removeDirectory(_tempStoragePath);
            CC_SAFE_RELEASE(_tempManifest);
            // Recreate temp storage path and save remote manifest
            _fileUtils->createDirectory(_tempStoragePath);
            _remoteManifest->saveToFile(_tempManifestPath);
        }

std::string exsitedPath = _storagePath + diff.asset.path;
_fileUtils->removeFile(exsitedPath);
// std::string exsitedPath = _storagePath + diff.asset.path;
// _fileUtils->removeFile(exsitedPath);
Copy link
Contributor

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.

理解了,判断条件取反只保留 else 分支把

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是的,我这边没有移除,只是注释掉,这部分代码移动了 AssetsManagerEx::updateSucceed 里面完成了,保证热更新未完成前,原有_storagePath里面的资源是完整的。

Copy link
Contributor

Choose a reason for hiding this comment

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

代码整理干净一些,没必要保留这个判断分支

if (diff.type != Manifest::DiffType::DELETED)
{
    ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的

@holycanvas holycanvas changed the base branch from v2.3.3-release to v2.4.0 April 22, 2020 13:19
@holycanvas holycanvas merged commit 4a08745 into cocos:v2.4.0 Apr 22, 2020
@zPiggy
Copy link

zPiggy commented May 12, 2020

下载完成后,文件从临时目录拷贝的时候会造成卡顿现象,放到线程中执行会不会更好些

@@ -885,7 +885,7 @@ void AssetsManagerEx::prepareUpdate()
std::string path = diff.asset.path;
DownloadUnit unit;
unit.customId = it->first;
unit.srcUrl = packageUrl + path;
unit.srcUrl = packageUrl + path + "?md5=" + diff.asset.md5;
Copy link

Choose a reason for hiding this comment

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

由于 md5 通常以 ‘==’ 结尾 或含有其他多义字符,是否encode更安全些

Copy link
Contributor Author

Choose a reason for hiding this comment

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

md5值是hex字符串,其计算方式是下面这样的,你是怎么遇到你描述的情况的,有测试工程吗?
crypto.createHash('md5').update(fs.readFileSync(subpath)).digest('hex');

Copy link

Choose a reason for hiding this comment

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

那没有问题了

Copy link

Choose a reason for hiding this comment

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

https://docs.cocos.com/creator/manual/zh/advanced-topics/assets-manager.html#manifest-%E6%A0%BC%E5%BC%8F

按照上述文档,

md5 信息可以不是文件的 md5 码,也可以是某个版本号,这完全是由用户决定的,本地和远程 manifest 对比时,只要 md5 信息不同,我们就认为这个文件有改动。

所以encode还是更安全的选择。

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.

None yet

5 participants