-
Notifications
You must be signed in to change notification settings - Fork 10
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
ZenKit migration issues #90
Comments
Hio @Try , I'll investigate point 3 and I'll get back to you on that. As for point 1: This change has been introduced for better compatibility with the original format and to allow saving the VOb-tree using ZenKit. Internally, I now track Point 2 falls into the same category: As I understand it, the object ID is purely there for resolving references in the archive. It is unreliable, because it's value is not guaranteed to be the same if you were to re-compile the world and it is not used anywhere in the ZenGin to uniquely identify object, as far as I can tell. If you need to compare objects, you can now just pointer-compare, since all of the reference resolution happens inside ZenKit and you just get a |
.1 This causes .2
OpenGothic already uses it for save/load purpose; void Vob::save(Serialize& fout) const {
fout.setEntry("worlds/",fout.worldName(),"/mobsi/",vobObjectID,"/data");
fout.write(uint8_t(vobType),pos,local);
} Removing Also kind remainder, that loading time get progressively slower and slower with updates. It can be due to overuse of small-dynamic allocations, but need to investigate. |
Updated first comment: |
…ternal` Capturing a temporary to a callback handler in the wrapper causes a dangling reference if `callback` is not held anywhere else.
Hm, I don't really have a good solution I think. Right now the order of operations would be:
Since asset files presumably exist if the object has a visual, the "asset not found" case is exceptional and should thus probably be handled separately.
True, that's a bigger issue than I thought. My proposal would be: change the save files so that the entire VOb tree is saved, then you can simply load it entirely from the save. This is what the original does. It should also improve reliability and could make loading faster, since you no longer have to read in the original world file. For now though, I'd say: let's keep the deprecated API around for the foreseeable future. I'd still like to mark it as deprecated so as to discourage others from using it but this legacy use-case I'll continue to support for now if that works for you. Same goes for the first point. If you want to continue to use the old API, go for it. Just tell me about it so I don't remove before coordinating with you.
Yes, performance improvements are on my bucket list (see also #88). It's not a high priority for me right now though.
Fixed in 3d67ed5. |
Can you provide a dummy visual, and point to it?
Backward compatibility :( Side topic: would it be possible to introduce stream-reader at some point? Shame to parse full-world into ram, only to convert it immediately to internal representation. |
I could, though I would lock it behind a compile-time setting if that's okay with you.
You can use use You can use
You could also just skip the conversion step and use the ZenKit datastructures directly. Since you can write to them, it should be possible to only use OpenGothic's structures for data not already represented in the VOb abstraction. For a basic example on how to use In all of these cases though, you will have to re-implement the world wrapper parsing yourself, the mesh, BSP-tree and waynet can be parsed on their own. See this: Lines 98 to 201 in 89af81c
|
Hi @lmichaelis ! Fix works only partially: visual is still null with |
True, I forgot about that :D Updated in 35938c2. |
Building now with |
C:/Programming/OpenGothic/lib/ZenKit/include/zenkit/ModelAnimation.hh:163:84: note: declared here
163 | ZKREM("renamed to Animation::sample_position_scale") float sample_position_scalar {};
| ^~~~~~~~~~~~~~~~~~~~~~
C:/Programming/OpenGothic/game/graphics/mesh/animation.cpp: In constructor 'Animation::Sequence::Sequence(const zenkit::MdsAnimation&, std::string_view)':
C:/Programming/OpenGothic/game/graphics/mesh/animation.cpp:197:26: note: synthesized method 'zenkit::ModelAnimation::ModelAnimation()' first required here
197 | zenkit::ModelAnimation p; mingw doesn't like initialization of deprecated variables... changing it to code bellow resolves the issue: union {
ZKREM("renamed to Animation::sample_position_min") float sample_position_range_min;
float sample_position_min {};
};
Just a minor note: |
VirtualObject::visual
can be null after loading. This makes usage unreasonable complex, forcing extra NPE-check.VParticleEffectController
,VMover
are among of the cases when both null and non-null may occur.VirtualObject::id
is essential part of save files, but now deprecatedset(ZK_ENABLE_DEPRECATION OFF CACHE INTERNAL "")
has no effect anymore - wasn't able to track why exactly it is-- 16.04.2024:
size+1 error in multiple places:
This is not complete list, rather some immediate observations to start discussion.
The text was updated successfully, but these errors were encountered: