[Minor] Maintain compatibility withold tags#2093
[Minor] Maintain compatibility withold tags#2093TaranDahl merged 18 commits intoPhobos-developers:developfrom
Conversation
DeathFishAtEase
added
the
No Documentation Needed
|
Nightly build for this pull request:
This comment is automatic and is meant to allow guests to get latest nightly builds for this pull request without registering. It is updated on every successful build. |
TaranDahl
left a comment
TaranDahl
left a comment
There was a problem hiding this comment.
I think you should add comments near the patch to facilitate future deletion.
TaranDahl
added
the
No test needed
|
@Metadorius I think there's no problem with this. Can it be merged now? |
Metadorius
left a comment
Metadorius
left a comment
There was a problem hiding this comment.
I still think that old tag names should be read with a log warning emitted. And the proper tag names should be prioritized, not vice versa.
As I understand it, it reads the old name first and then the new name. So, it should give priority to using the new name. |
Right, I think I've seen what @Coronia commented on and was thrown off because of this. |
TaranDahl
added
the
Will be merged in 24h
|
To Chinese users:
|
Does anyone know why repeated loading is necessary? I discovered this while handling #2093 and suspect it is a bug. By switching to the `develop` branch and adding log in `WarheadTypeExt::ExtData::LoadFromINIFile`, it can be observed that each warhead is loaded twice. For example, based on 6035b7b, make the following modifications: ```diff DEFINE_HOOK(0x75DEA0, WarheadTypeClass_LoadFromINI, 0x5) { GET(WarheadTypeClass*, pItem, ESI); GET_STACK(CCINIClass*, pINI, 0x150); + static std::map<uintptr_t, int> iniCallCount; + uintptr_t iniAddr = reinterpret_cast<uintptr_t>(pINI); + int count = ++iniCallCount[iniAddr]; + Debug::Log("[DEBUG] LoadFromINIFile for [%s] count: %d\n", pItem->ID, count); WarheadTypeExt::ExtMap.LoadFromINI(pItem, pINI); return 0; } ``` In `debug.log`, it can be observed: <img width="1948" height="914" alt="image" src="https://github.com/user-attachments/assets/63ac1732-9103-444f-88a2-68634fcac5cd" /> --------- Co-authored-by: Netsu_Negi <[email protected]> Co-authored-by: Coronia <[email protected]>
5 participants
This is a temporary solution supplementing #1984.
Important
It is inelegant and inconvenient to maintain, but we need at least one workable solution ready before the next devbuild release, even if it will eventually be entirely replaced by other implementations.
There have been complaints about what the tag renaming PR did:
Because the Phobos-chat channel keeps mentioning these issues, and maintainers like Ollerus are also concerned about the impact of whether to roll back on users already using the new or old versions, this PR, which was completed long ago but is obviously not perfect, has been opened here—at least when we have to face this problem, it is an option available to us.
Caution
The branch of this PR will be synced with
developrelatively frequently, but timely updates to the latest state of thedevelopbranch are not guaranteed.