 | OpenMW Development | 745 Members |
| OpenMW project channel. Discussion of topics loosely related to the project development. || Main channel: #openmw:matrix.org | 5 Servers |
| OpenMW Development | 745 Members |
| OpenMW project channel. Discussion of topics loosely related to the project development. || Main channel: #openmw:matrix.org | 5 Servers |
| Sender | Message | Time |
|---|---|---|
| 1 Aug 2021 | ||
 cc9cii | So, I spent the last little while trying to implement the "one reader" for ESM3 and ESM4 file types. I thought about placing this note in [Issue #6176](https://gitlab.com/OpenMW/openmw/-/issues/6176) but I don't think implementation notes belong in Issues. Probably best to put such in an MR, but I'm not sure there should be an MR to begin with.The TL;DR is that the whole premise, i.e. "one reader", may not make sense. Why one reader? I would imagine the basis for wanting "one reader" would be to reduce code duplication and to ease future maintenance. Why maintain two if you can maintain just one? What are the duplicated/common code blocks? Surprisingly, there isn't much. The file/stream open() and close() methods, and low level data access.What are the differences? There are many. * ESM3 or ESM4 header for the file/stream* file context structure (e.g. ESM4 context require more info)* record/sub-record header sizes and structures (which are required to get the next record/sub-record) * navigating through the records (e.g. the concept of nested "groups" in ESM4)* fail() method to report errors (different structures require different implementation)Are there any costs in implementing one reader? Performance - without resorting to rather complicated set o template classes, we end up with vtable lookups in many of the often used method calls. Note complicated templates will hinder the original aim of ease of maintenance. Note2: profiling has not been done, so I'm unable to quantify the performance hit at this point. Complexity - even with the introduction of a base class (and resulting vtables) we can't avoid complexity with template methods - e.g. to get/set file context for the different ESM typesSo what are the alternatives? Continued below due to text limit. | 01:03:09 |
| cc9cii | The obvious alternative is to have two separate readers. So don't do anything? Not quite.. * There is still scope to increase maintainability. The two readers that currently exist are written in different style and the way the readers are used (i.e. the API) are different. * It should be possible to reduce code duplication by declaring some methods in anonymous namespace (suspect there will be many different ways to do this). | 01:12:29 |
| cc9cii | The obvious alternative is to have two separate readers. So don't do anything? Not quite. We can still meet some of the original aim: * There is still scope to increase maintainability. The two readers that currently exist are written in different style and the way the readers are used (i.e. the API) are different. * It should be possible to reduce code duplication by declaring some methods in anonymous namespace (suspect there will be many different ways to do this). | 01:13:48 |
| cc9cii | The obvious alternative is to have two separate readers. So don't do anything? Not quite. We can still meet some of the original aim: * There is still scope to increase maintainability. The two readers that currently exist are written in different style and the way the readers are used (i.e. the API) are different. * It should be possible to reduce code duplication by declaring some methods in anonymous namespace (suspect there will be many different ways to do this). Which reader then? Well, I can't be objective here. | 01:14:43 |
 jvoisin#8882 | Implementation notes might belong in issues | 01:15:28 |
| jvoisin#8882 | Instead of being lost in the discord limbo :) | 01:15:38 |
| cc9cii | maybe I park it there until an MR is raised? | 01:16:51 |
| jvoisin#8882 | Sure | 01:17:05 |
| jvoisin#8882 | We can always edit the issue :) | 01:17:22 |
 Peleus joined the room. | 01:18:02 | |
| cc9cii | I copied above note to Issue #6176. | 01:22:29 |
| cc9cii | The obvious alternative is to have two separate readers. So don't do anything? Not quite. We can still meet some of the original aim: * There is still scope to increase maintainability. The two readers that currently exist are written in different style and the way the readers are used (i.e. the API) are different. By having a common (and possibly more modern C++) we can increase maintainability. * It should be possible to reduce code duplication by declaring some methods in anonymous namespace (suspect there will be many different ways to do this). Which reader then? Well, I can't be objective here. | 01:23:19 |
| cc9cii | * So, I spent the last little while trying to implement the "one reader" for ESM3 and ESM4 file types. I thought about placing this note in [Issue #6176](https://gitlab.com/OpenMW/openmw/-/issues/6176) but I don't think implementation notes belong in Issues. Probably best to put such in an MR, but I'm not sure there should be an MR to begin with.The TL;DR is that the whole premise, i.e. "one reader", may not make sense. Why one reader? I would imagine the basis for wanting "one reader" would be to reduce code duplication and to ease future maintenance. Why maintain two if you can maintain just one? What are the duplicated/common code blocks? Surprisingly, there isn't much. The file/stream open() and close() methods, and low level data access.What are the differences? There are many. * ESM3 or ESM4 header for the file/stream* file context structure (e.g. ESM4 context require more info)* record/sub-record header sizes and structures (which are required to get the next record/sub-record) * navigating through the records (e.g. the concept of nested "groups" in ESM4)* fail() method to report errors (different structures require different implementation)Are there any costs in implementing one reader? Performance - without resorting to rather complicated set o template classes, we end up with vtable lookups in many of the often used method calls. Note complicated templates will hinder the original aim of ease of maintenance. Note2: profiling has not been done, so I'm unable to quantify the performance hit at this point. Complexity - even with the introduction of a base class (and resulting vtables) we can't avoid complexity with template methods - e.g. to get/set file context for the different ESM typesSo what are the alternatives? Continued below due to text limit. | 01:25:27 |
| cc9cii | * The obvious alternative is to have two separate readers. So don't do anything? Not quite. We can still meet some of the original aim: * There is still scope to increase maintainability. The two readers that currently exist are written in different style and the way the readers are used (i.e. the API) are different. By having a common (and possibly more modern C++) we can increase maintainability. * It should be possible to reduce code duplication by declaring some methods in anonymous namespace (suspect there will be many different ways to do this). Which reader then? Well, I can't be objective here. | 01:30:14 |
 CMAugust#6575 | I've heard a report of stuttering on crossing a cell that is present in latest builds but not 0.47.0, even when object paging is turned off. Is that the case for anyone else? | 01:56:33 |
 eddie5 | Has been. IDK about 0.47. And I can't even say that that it makes the load take longer. | 01:58:30 |
| CMAugust#6575 | The user in question has a weaker PC than mine, so I'm having trouble reproducing it on my end. | 02:02:30 |
| CMAugust#6575 | Update: unchecking navmesh generation in the launcher fixed his stuttering. | 02:03:17 |
| eddie5 | I haven't noticed it so much since I set the distance from player or whatev to 1 or 0 - I forget. | 02:04:14 |
| eddie5 | 1. | 02:04:49 |
 dexasz#6961 changed their profile picture. | 02:30:54 | |
 Isaiah 1 11#7054 | Depends I heard some SHOTN stuff can cause navmesh generation to get a little…funky | 02:53:34 |
 Arbok joined the room. | 07:00:11 | |
 _discord_777709614329757696_=44arth=47andalf | cc9cii Implementation notes might belong in code comments | 07:21:16 |
| cc9cii | you're probably right, but we don't do that (as in, notes such as above) in our codebase, - and in any case I'm explaining why I'm not writing some code so there's no convenient place to put the notes | 07:23:10 |
| cc9cii | * you're probably right, but we don't do that (as in, notes such as above) in our codebase, - and in any case I'm explaining why I'm not writing some code so there's no convenient place to put the notes | 07:27:22 |
 Sinscere joined the room. | 08:11:40 | |
 psi29a#3708 | Replied with an alternative path | 09:05:57 |
| cc9cii | Thanks, I also replied, since I didn't quite understand the alternative | 09:26:32 |
| psi29a#3708 | Good thing we're talking it out 😉 | 10:33:52 |