Fixed breaking change to animated clock

[Tugark]

Hi Son

Following your comment https://github.com/3dcitydb/3dcitydb-web-map/pull/28#issuecomment-386011554 on my last pull-request, I fixed the issue with the clock. It should now animate once again.

As for your other questions:

• Regarding documentation: No, I meant with all the objects itself; so for example which properties the Webmapcity Object has, which methods the Citydblayers have, etc. If I'm too blind to see such a documentation, please let me know =)
• Regarding geoproregio: Yes, I'm working there as a developer.

Best, Lukas

[Son-HNguyen]

Hi Lukas,

I think it would be better to call the clock using the cesiumViewer.cesiumWidget.clock. You can change lines 172 - 179 of script.js to something like this:

var clock = cesiumViewer.cesiumWidget.clock;
clock.shouldAnimate = true;
var dayTimeStr = CitydbUtil.parse_query_string('dayTime', window.location.href);
if (dayTimeStr) {
    var julianDate = Cesium.JulianDate.fromIso8601(decodeURIComponent(dayTimeStr));
    clock.currentTime = julianDate;
    clock.shouldAnimate = false;
}

Regarding the documentation of the 3DCityDB JavaScript classes: That is something we can definitely add to the project to provide users (especially power users and developers) a better understanding of the tool. But since we currently do not have the resources to do such thing, you are more than welcome to contribute. 👍

Best, Son

[Tugark]

Hi Son

Why do you think that way works better? I've been using it my way in one of my projects and have not had any issues yet, but I'm curious to know what the advantage of your approach over mine ist :-)

Best,

Lukas

[Son-HNguyen]

Hi Lukas,

I prefer calling the cesiumViewer.cesiumWidget.clock to creating a new clock because the former does not replace an existing clock with a new one like the latter does (see documentation). If for example you modified the clock somewhere in the code, calling the constructor would lose all these modifications. Of course in our case both will do the same since they are called upon init, but I think it's safer to use cesiumViewer.cesiumWidget.clock. I also tend to keep related codes near each other so I can find them easily later. In the end, it is more or less a matter of taste. ;-)

Best, Son

[Tugark]

Hi Son

I see your point. However, in that particular case, I think it would make sense to put the Clock definition to the webmap init. As you say, we should keep related code together, yet in my opinion, that would also mean that we should keep Initcode together.

In script.js, you also parse the shadow and terrainshadows in the beginning, since they are used for constructing the Cesium instance. If we have the Clock at the start, we can define the "basic clock" and have all related configuration there (especially if they make more changes to Clock); in the later parts of the code we should of course use the widget to not override our basic clock. Another advantage would be that we define the Clock explicitly; using the code as it was now just uses the Cesium defaults, which can lead to that kind of issues that we have now (i.e. Cesium changing the default behaviour).

As such, we would have the basic configuration with shadows, clock and other Init-related code at the start (where we also initiate the viewer), and everything else that follows is placed behind that part.

However, that's just my opinion :-) If you'd rather like to use the CesiumWidget, I'll test your suggestion and put it there.

Best, Lukas

[Son-HNguyen]

Hi Lukas,

I see. Your approach is better used for scenarios where we have to set up a clock manually and use it (i.e. the global clock variable) multiple times, but since we only want to set the shouldAnimate attribute, any of the proposed codes would suffice. I will accept the pull request now (unless you'd like to commit any changes) so we can move on with the other fixes.

Best, Son

[Tugark]

Hi Son

Sounds good to me :-)

As for the documentation mentioned above, I'll do my best to create something in that respect. However, I'm currently focussing on getting that webpack version running. (Which does run, by the way, however only with my modified script. I'll work on building the "script.js" from the official repo, so we can test the webpack version better. I hope we'll be able to get that npm install 3dcitydb-webclient going =)).

Best,

Lukas

[Son-HNguyen]

Hi Lukas,

regarding the Webpack, the 3DCityDB Web Map Client also has a docker image, which you can find on our chair's homepage. Perhaps you'll find this useful.

Best, Son